commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Kitching <skitch...@apache.org>
Subject Re: svn commit: r395181 - /jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
Date Wed, 19 Apr 2006 09:09:33 GMT
Hi Robert,

As you can see I've had a go at fixing the odd diagnostic message. I'd
be grateful if you could double-check this sometime.

====
The initial code did:
  // match package plus first 4 chars
  if (candidate.regionMatches(true, 0, name, 0, 38)) {...}
I presume the "38" was meant to be
  length("org.apache.commons.logging.impl.") + 4.
However that works out to be 32+4=36. So the old code was actually
testing the first 6 chars.

I decided to make the code a little more self-explanatory (defined
constants etc) and went with the 4 chars as stated by the comment,
rather than the 4 chars as implemented.
====
I also switched from candidate.regionMatches(true, 0, name, ..) to
name.regionMatches(true, 0, candidate, ...). Actually, on hindsight that
wasn't necessary; I had another test in there for a while which used
name and therefore made them all consistent. However 
  x.regionMatches(.., y, ..) and
  y.regionMatches(.., x, ..) 
are both the same, yes? (as long as x and y are both non-null which they
always are).
====
Also, rather than trimming the user-specified classname only before
doing diagnostic output, the name is now trimmed within the
findUserSpecifiedLogClassName.

Cheers,

Simon

On Wed, 2006-04-19 at 08:57 +0000, skitching@apache.org wrote:
> Author: skitching
> Date: Wed Apr 19 01:57:54 2006
> New Revision: 395181
> 
> URL: http://svn.apache.org/viewcvs?rev=395181&view=rev
> Log:
> Fix problem with "suggested" alternative for invalid log adapter class.
> Always trim whitespace from user-specified log adapter class name.
> 
> Modified:
>     jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
> 
> Modified: jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
> URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java?rev=395181&r1=395180&r2=395181&view=diff
> ==============================================================================
> --- jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
(original)
> +++ jakarta/commons/proper/logging/trunk/src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
Wed Apr 19 01:57:54 2006
> @@ -77,6 +77,8 @@
>      /** SimpleLog class name */
>      private static final String LOGGING_IMPL_SIMPLE_LOGGER = "org.apache.commons.logging.impl.SimpleLog";
>  
> +    private static final String PKG_IMPL="org.apache.commons.logging.impl.";
> +    private static final int PKG_LEN = PKG_IMPL.length();
>      
>      // ----------------------------------------------------------- Constructors
>  
> @@ -783,15 +785,13 @@
>                  messageBuffer.append(specifiedLogClassName);
>                  messageBuffer.append("' cannot be found or is not useable.");
>                  
> -                //
>                  // Mistyping or misspelling names is a common fault.
>                  // Construct a good error message, if we can
>                  if (specifiedLogClassName != null) {
> -                    final String trimmedName = specifiedLogClassName.trim();
> -                    informUponSimilarName(messageBuffer, trimmedName, LOGGING_IMPL_LOG4J_LOGGER);
> -                    informUponSimilarName(messageBuffer, trimmedName, LOGGING_IMPL_JDK14_LOGGER);
> -                    informUponSimilarName(messageBuffer, trimmedName, LOGGING_IMPL_LUMBERJACK_LOGGER);
> -                    informUponSimilarName(messageBuffer, trimmedName, LOGGING_IMPL_SIMPLE_LOGGER);
> +                    informUponSimilarName(messageBuffer, specifiedLogClassName, LOGGING_IMPL_LOG4J_LOGGER);
> +                    informUponSimilarName(messageBuffer, specifiedLogClassName, LOGGING_IMPL_JDK14_LOGGER);
> +                    informUponSimilarName(messageBuffer, specifiedLogClassName, LOGGING_IMPL_LUMBERJACK_LOGGER);
> +                    informUponSimilarName(messageBuffer, specifiedLogClassName, LOGGING_IMPL_SIMPLE_LOGGER);
>                  }
>                  throw new LogConfigurationException(messageBuffer.toString());
>              }
> @@ -845,19 +845,25 @@
>      }
>  
> 
> -    
>      /**
>       * Appends message if the given name is similar to the candidate.
>       * @param messageBuffer <code>StringBuffer</code> the message should
be appended to, 
>       * not null
>       * @param name the (trimmed) name to be test against the candidate, not null
> -     * @param candidate the candidate name 
> +     * @param candidate the candidate name (not null)
>       */
>      private void informUponSimilarName(final StringBuffer messageBuffer, final String
name, 
>              final String candidate) {
> -        // this formular (first four letters of the name excluding package) 
> -        // gives a reason guess
> -        if (candidate.regionMatches(true, 0, name, 0, 38)) {
> +        if (name.equals(candidate)) {
> +            // Don't suggest a name that is exactly the same as the one the
> +            // user tried...
> +            return;
> +        }
> +
> +        // If the user provides a name that is in the right package, and gets
> +        // the first 4 characters of the adapter class right (ignoring case),
> +        // then suggest the candidate adapter class name.
> +        if (name.regionMatches(true, 0, candidate, 0, PKG_LEN + 4)) {
>              messageBuffer.append(" Did you mean '");
>              messageBuffer.append(candidate);
>              messageBuffer.append("'?");
> @@ -917,8 +923,14 @@
>              }
>          }
>          
> +        // Remove any whitespace; it's never valid in a classname so its
> +        // presence just means a user mistake. As we know what they meant,
> +        // we may as well strip the spaces.
> +        if (specifiedClass != null) {
> +            specifiedClass = specifiedClass.trim();
> +        }
> +
>          return specifiedClass;
> -        
>      }
>  
>      
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> 


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


Mime
View raw message