logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: svn commit: r1594250 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java
Date Wed, 14 May 2014 16:04:37 GMT
I can't tell any of these apart on my phone without zooming in close!

Same when I'm in Eclipse. I refuse to code in courier. Java is not cobol! ;)

Gary

<div>-------- Original message --------</div><div>From: Matt Sicker <boards@gmail.com>
</div><div>Date:05/14/2014  11:59  (GMT-05:00) </div><div>To: Log4J
Developers List <log4j-dev@logging.apache.org> </div><div>Subject: Re: svn
commit: r1594250 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java
</div><div>
</div>I think it'd be important to make named constants for things like "'" or '"' (wow
those look impossible to read in sans-serif). For an empty string, the "" is the constant.


On 14 May 2014 10:54, Ralph Goers <ralph.goers@dslextreme.com> wrote:
Any time your reason for a change ends with “IMO” I would recommend asking first.  In
this case it seems like hundreds of instances were changed, which would seem to me that the
various people who did that thought differently than you.  

Ralph

On May 14, 2014, at 6:34 AM, Gary Gregory <garydgregory@gmail.com> wrote:

It's not always obvious, for example, when you see someVar = "" and otherVar = "'". Are they
both right, does one have a typo?

It's about communicating intent. Using a constant leaves no room for misinterpretation.

Maybe readers that use a giant courier font "see" it differently, but the point of communicating
intent still holds IMO.

Gary


On Wed, May 14, 2014 at 3:10 AM, Ralph Goers <ralph.goers@dslextreme.com> wrote:
Do these changes really accomplish anything?  What is so magic about ""? It is pretty damn
obvious what is going on.

Ralph

On May 13, 2014, at 8:35 AM, ggregory@apache.org wrote:

> Author: ggregory
> Date: Tue May 13 15:35:38 2014
> New Revision: 1594250
>
> URL: http://svn.apache.org/r1594250
> Log:
> Refactor magic strings into a constant.
>
> Modified:
>    logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java
>
> Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java
> URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java?rev=1594250&r1=1594249&r2=1594250&view=diff
> ==============================================================================
> --- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java
(original)
> +++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/jmx/LoggerContextAdmin.java
Tue May 13 15:35:38 2014
> @@ -47,6 +47,7 @@ import org.apache.logging.log4j.core.hel
> import org.apache.logging.log4j.core.helpers.Closer;
> import org.apache.logging.log4j.core.helpers.FileUtils;
> import org.apache.logging.log4j.status.StatusLogger;
> +import org.apache.logging.log4j.util.Strings;
>
> /**
>  * Implementation of the {@code LoggerContextAdminMBean} interface.
> @@ -113,7 +114,7 @@ public class LoggerContextAdmin extends
>         if (getConfigName() != null) {
>             return String.valueOf(new File(getConfigName()).toURI());
>         }
> -        return "";
> +        return Strings.EMPTY;
>     }
>
>     @Override
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org




-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org 
Java Persistence with Hibernate, Second Edition
JUnit in Action, Second Edition
Spring Batch in Action
Blog: http://garygregory.wordpress.com 
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory




-- 
Matt Sicker <boards@gmail.com>
Mime
View raw message