No need to nest in else.

src/main/java/org/apache/commons/logging/LogFactory.java

@@ -878,9 +878,8 @@ public abstract class LogFactory {
             //
             // nb: nullClassLoaderFactory might be null. That's ok.
             return nullClassLoaderFactory;
-        } else {
-            return (LogFactory) factories.get(contextClassLoader);
         }
+        return (LogFactory) factories.get(contextClassLoader);
     }
 
     /**
@@ -1239,9 +1238,8 @@ public abstract class LogFactory {
                 public Object run() {
                     if (loader != null) {
                         return loader.getResourceAsStream(name);
-                    } else {
-                        return ClassLoader.getSystemResourceAsStream(name);
                     }
+                    return ClassLoader.getSystemResourceAsStream(name);
                 }
             });
     }
@@ -1267,9 +1265,8 @@ public abstract class LogFactory {
                     try {
                         if (loader != null) {
                             return loader.getResources(name);
-                        } else {
-                            return ClassLoader.getSystemResources(name);
                         }
+                        return ClassLoader.getSystemResources(name);
                     } catch (final IOException e) {
                         if (isDiagnosticsEnabled()) {
                             logDiagnostic("Exception while trying to find configuration file " +
@@ -1475,17 +1472,17 @@ public abstract class LogFactory {
 
         if (dest.equals("STDOUT")) {
             return System.out;
-        } else if (dest.equals("STDERR")) {
+        }
+        if (dest.equals("STDERR")) {
             return System.err;
-        } else {
+        }
-            try {
+        try {
-                // open the file in append mode
+            // open the file in append mode
-                final FileOutputStream fos = new FileOutputStream(dest, true);
+            final FileOutputStream fos = new FileOutputStream(dest, true);
-                return new PrintStream(fos);
+            return new PrintStream(fos);
-            } catch (final IOException ex) {
+        } catch (final IOException ex) {
-                // We should report this to the user - but how?
+            // We should report this to the user - but how?
-                return null;
+            return null;
-            }
         }
     }
 
@@ -1651,9 +1648,8 @@ public abstract class LogFactory {
     public static String objectId(final Object o) {
         if (o == null) {
             return "null";
-        } else {
-            return o.getClass().getName() + "@" + System.identityHashCode(o);
         }
+        return o.getClass().getName() + "@" + System.identityHashCode(o);
     }
 
     // ----------------------------------------------------------------------

src/main/java/org/apache/commons/logging/impl/LogFactoryImpl.java

@@ -682,12 +682,11 @@ public class LogFactoryImpl extends LogFactory {
                     logDiagnostic("Did not find '" + name + "'.");
                 }
                 return false;
-            } else {
-                if (isDiagnosticsEnabled()) {
-                    logDiagnostic("Found '" + name + "'.");
-                }
-                return true;
             }
+            if (isDiagnosticsEnabled()) {
+                logDiagnostic("Found '" + name + "'.");
+            }
+            return true;
         } catch (final LogConfigurationException e) {
             if (isDiagnosticsEnabled()) {
                 logDiagnostic("Logging system '" + name + "' is available but not useable.");
@@ -1166,22 +1165,20 @@ public class LogFactoryImpl extends LogFactory {
            // In some classloading setups (e.g. JBoss with its
            // UnifiedLoaderRepository) this can still work, so if user hasn't
            // forbidden it, just return the contextClassLoader.
-           if (allowFlawedContext) {
+           if (!allowFlawedContext) {
-              if (isDiagnosticsEnabled()) {
-                   logDiagnostic("[WARNING] the context classloader is not part of a" +
-                                 " parent-child relationship with the classloader that" +
-                                 " loaded LogFactoryImpl.");
-              }
-              // If contextClassLoader were null, getLowestClassLoader() would
-              // have returned thisClassLoader.  The fact we are here means
-              // contextClassLoader is not null, so we can just return it.
-              return contextClassLoader;
-           }
-           else {
             throw new LogConfigurationException("Bad classloader hierarchy; LogFactoryImpl was loaded via" +
                                                 " a classloader that is not related to the current context" +
                                                 " classloader.");
            }
+        if (isDiagnosticsEnabled()) {
+               logDiagnostic("[WARNING] the context classloader is not part of a" +
+                             " parent-child relationship with the classloader that" +
+                             " loaded LogFactoryImpl.");
+          }
+          // If contextClassLoader were null, getLowestClassLoader() would
+          // have returned thisClassLoader.  The fact we are here means
+          // contextClassLoader is not null, so we can just return it.
+          return contextClassLoader;
         }
 
         if (baseClassLoader != contextClassLoader) {
@@ -1190,21 +1187,20 @@ public class LogFactoryImpl extends LogFactory {
             // that there are a number of broken systems out there which create
             // custom classloaders but fail to set the context classloader so
             // we handle those flawed systems anyway.
-            if (allowFlawedContext) {
+            if (!allowFlawedContext) {
-                if (isDiagnosticsEnabled()) {
-                    logDiagnostic(
-                            "Warning: the context classloader is an ancestor of the" +
-                            " classloader that loaded LogFactoryImpl; it should be" +
-                            " the same or a descendant. The application using" +
-                            " commons-logging should ensure the context classloader" +
-                            " is used correctly.");
-                }
-            } else {
                 throw new LogConfigurationException(
                         "Bad classloader hierarchy; LogFactoryImpl was loaded via" +
                         " a classloader that is not related to the current context" +
                         " classloader.");
             }
+            if (isDiagnosticsEnabled()) {
+                logDiagnostic(
+                        "Warning: the context classloader is an ancestor of the" +
+                        " classloader that loaded LogFactoryImpl; it should be" +
+                        " the same or a descendant. The application using" +
+                        " commons-logging should ensure the context classloader" +
+                        " is used correctly.");
+            }
         }
 
         return baseClassLoader;

src/main/java/org/apache/commons/logging/impl/SimpleLog.java

@@ -633,9 +633,7 @@ public class SimpleLog implements Log, Serializable {
                  * obtain a class loader) will trigger this exception where
                  * we can make a distinction.
                  */
-                if (e.getTargetException() instanceof SecurityException) {
+                if (!(e.getTargetException() instanceof SecurityException)) {
-                    // ignore
-                } else {
                     // Capture 'e.getTargetException()' exception for details
                     // alternate: log 'e.getTargetException()', and pass back 'e'.
                     throw new LogConfigurationException
@@ -664,9 +662,8 @@ public class SimpleLog implements Log, Serializable {
 
                     if (threadCL != null) {
                         return threadCL.getResourceAsStream(name);
-                    } else {
-                        return ClassLoader.getSystemResourceAsStream(name);
                     }
+                    return ClassLoader.getSystemResourceAsStream(name);
                 }
             });
     }

src/test/java/org/apache/commons/logging/PathableClassLoader.java

@@ -312,26 +312,18 @@ public class PathableClassLoader extends URLClassLoader {
 
         if (parentFirst) {
             return super.loadClass(name, resolve);
-        } else {
+        }
-            // Implement child-first.
+        try {
-            //
+            Class clazz = findLoadedClass(name);
-            // It appears that the findClass method doesn't check whether the
+            if (clazz == null) {
-            // class has already been loaded. This seems odd to me, but without
+                clazz = super.findClass(name);
-            // first checking via findLoadedClass we can get java.lang.LinkageError
-            // with message "duplicate class definition" which isn't good.
-
-            try {
-                Class clazz = findLoadedClass(name);
-                if (clazz == null) {
-                    clazz = super.findClass(name);
-                }
-                if (resolve) {
-                    resolveClass(clazz);
-                }
-                return clazz;
-            } catch(final ClassNotFoundException e) {
-                return super.loadClass(name, resolve);
             }
+            if (resolve) {
+                resolveClass(clazz);
+            }
+            return clazz;
+        } catch(final ClassNotFoundException e) {
+            return super.loadClass(name, resolve);
         }
     }
 
@@ -344,13 +336,12 @@ public class PathableClassLoader extends URLClassLoader {
     public URL getResource(final String name) {
         if (parentFirst) {
             return super.getResource(name);
-        } else {
-            final URL local = super.findResource(name);
-            if (local != null) {
-                return local;
-            }
-            return super.getResource(name);
         }
+        final URL local = super.findResource(name);
+        if (local != null) {
+            return local;
+        }
+        return super.getResource(name);
     }
 
     /**
@@ -364,30 +355,29 @@ public class PathableClassLoader extends URLClassLoader {
     public Enumeration getResourcesInOrder(final String name) throws IOException {
         if (parentFirst) {
             return super.getResources(name);
-        } else {
-            final Enumeration localUrls = super.findResources(name);
-
-            final ClassLoader parent = getParent();
-            if (parent == null) {
-                // Alas, there is no method to get matching resources
-                // from a null (BOOT) parent classloader. Calling
-                // ClassLoader.getSystemClassLoader isn't right. Maybe
-                // calling Class.class.getResources(name) would do?
-                //
-                // However for the purposes of unit tests, we can
-                // simply assume that no relevant resources are
-                // loadable from the parent; unit tests will never be
-                // putting any of their resources in a "boot" classloader
-                // path!
-                return localUrls;
-            }
-            final Enumeration parentUrls = parent.getResources(name);
-
-            final ArrayList localItems = toList(localUrls);
-            final ArrayList parentItems = toList(parentUrls);
-            localItems.addAll(parentItems);
-            return Collections.enumeration(localItems);
         }
+        final Enumeration localUrls = super.findResources(name);
+
+        final ClassLoader parent = getParent();
+        if (parent == null) {
+            // Alas, there is no method to get matching resources
+            // from a null (BOOT) parent classloader. Calling
+            // ClassLoader.getSystemClassLoader isn't right. Maybe
+            // calling Class.class.getResources(name) would do?
+            //
+            // However for the purposes of unit tests, we can
+            // simply assume that no relevant resources are
+            // loadable from the parent; unit tests will never be
+            // putting any of their resources in a "boot" classloader
+            // path!
+            return localUrls;
+        }
+        final Enumeration parentUrls = parent.getResources(name);
+
+        final ArrayList localItems = toList(localUrls);
+        final ArrayList parentItems = toList(parentUrls);
+        localItems.addAll(parentItems);
+        return Collections.enumeration(localItems);
     }
 
     /**
@@ -418,18 +408,17 @@ public class PathableClassLoader extends URLClassLoader {
     public InputStream getResourceAsStream(final String name) {
         if (parentFirst) {
             return super.getResourceAsStream(name);
-        } else {
-            final URL local = super.findResource(name);
-            if (local != null) {
-                try {
-                    return local.openStream();
-                } catch(final IOException e) {
-                    // TODO: check if this is right or whether we should
-                    // fall back to trying parent. The javadoc doesn't say...
-                    return null;
-                }
-            }
-            return super.getResourceAsStream(name);
         }
+        final URL local = super.findResource(name);
+        if (local != null) {
+            try {
+                return local.openStream();
+            } catch(final IOException e) {
+                // TODO: check if this is right or whether we should
+                // fall back to trying parent. The javadoc doesn't say...
+                return null;
+            }
+        }
+        return super.getResourceAsStream(name);
     }
 }

src/test/java/org/apache/commons/logging/impl/WeakHashtableTestCase.java

@@ -251,11 +251,10 @@ public class WeakHashtableTestCase extends TestCase {
             if(weakHashtable.get(new Long(1)) == null) {
                 break;
 
-            } else {
-                // create garbage:
-                final byte[] b =  new byte[bytz];
-                bytz = bytz * 2;
             }
+            // create garbage:
+            final byte[] b =  new byte[bytz];
+            bytz = bytz * 2;
         }
 
         // some JVMs seem to take a little time to put references on

src/test/java/org/apache/commons/logging/security/MockSecurityManager.java

@@ -116,7 +116,8 @@ public class MockSecurityManager extends SecurityManager {
                 // the call stack.
                 System.out.println("Access controller found: returning");
                 return;
-            } else if (cname.startsWith("java.")
+            }
+            if (cname.startsWith("java.")
                 || cname.startsWith("javax.")
                 || cname.startsWith("junit.")
                 || cname.startsWith("org.apache.tools.ant.")
@@ -133,13 +134,12 @@ public class MockSecurityManager extends SecurityManager {
                 System.out.println("Untrusted code [testcase] found");
                 throw new SecurityException("Untrusted code [testcase] found");
             } else if (cname.startsWith("org.apache.commons.logging.")) {
-                if (permissions.implies(p)) {
+                if (!permissions.implies(p)) {
-                    // Code here is trusted if the caller is trusted
-                    System.out.println("Permission in allowed set for JCL class");
-                } else {
                     System.out.println("Permission refused:" + p.getClass() + ":" + p);
                     throw new SecurityException("Permission refused:" + p.getClass() + ":" + p);
                 }
+                // Code here is trusted if the caller is trusted
+                System.out.println("Permission in allowed set for JCL class");
             } else {
                 // we found some code that is not trusted to perform this operation.
                 System.out.println("Unexpected code: permission refused:" + p.getClass() + ":" + p);