List of Exception Management Anti-Patterns and Code Smells

[ashishsureka]

Code smells are defined as symptoms in the program source code which are usually not bugs or technically incorrect but indicates a possible deeper problem. Anti-patterns are counterparts of design patterns and are defined as mistakes during software development that produces negative consequences and is ineffective or counter-productive. During program execution, error events can occur that disrupts the normal flow of the program. Programming languages provide exception handling mechanism to developers for handling errors and exception events.

I mined the source-code (JUnit 4.11) for automatically detecting 10 exception handling anti-patterns (https://today.java.net/article/2006/04/04/exception-handling-antipatterns). In this issue report, I list the exception handling anti-patterns and code-smells that I found in the source code. My experimental results demonstrate presence of various exception handling anti-patterns and throw light on their intensity. I believe my tool for automatic detection of anti-patterns in source code and the attached results can be useful to programmers by helping them correct mistakes and write effective code.

Mining Source Code for Automatically Discovering Exception Management Anti-Patterns and Code Smell

---

FILE NAME : JUnit\junit\extensions\TestSetup.java

METHOD NAME : protect() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\junit\extensions\TestSetup.java

METHOD NAME : setUp() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\junit\extensions\TestSetup.java

METHOD NAME : tearDown() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\junit\framework\JUnit4TestAdapterCache.java

METHOD NAME : testFailure(Failure failure) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\junit\framework\JUnit4TestAdapterCache.java

METHOD NAME : testFinished(Description description) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\junit\framework\JUnit4TestAdapterCache.java

METHOD NAME : testStarted(Description description) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\junit\framework\TestCase.java

METHOD NAME : setUp() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\junit\framework\TestCase.java

METHOD NAME : tearDown() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\junit\runner\BaseTestRunner.java

CATCH CLAUSE : catch (Exception e) { runFailed("Error: " + e.toString()); return null; }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\junit\runner\BaseTestRunner.java

CATCH CLAUSE : catch (Exception e) { clearStatus(); return new TestSuite(testClass); }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\junit\runner\BaseTestRunner.java

CATCH CLAUSE : catch (Exception IOException) { return stack; }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\junit\textui\TestRunner.java

CATCH CLAUSE : catch (Exception e) { }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\junit\textui\TestRunner.java

CATCH CLAUSE : catch (Exception e) { System.err.println(e.getMessage()); System.exit(EXCEPTION_EXIT); }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\junit\textui\TestRunner.java

METHOD NAME : start(String args[]) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\junit\textui\TestRunner.java

CATCH CLAUSE : catch (Exception e) { throw new Exception("Could not create and run test suite: " + e); }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\junit\textui\TestRunner.java

METHOD NAME : runSingleMethod(String testCase,String method,boolean wait) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\experimental\max\MaxCore.java

CATCH CLAUSE : catch (ClassNotFoundException e) { return null; }

ANTI-PATERN: just returns null instead of handling or re-throwing the exception, swallows the exception, losing the information forever

---

---

FILE NAME : JUnit\org\junit\experimental\max\MaxHistory.java

CATCH CLAUSE : catch (Exception e) { throw new CouldNotReadCoreException(e); }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\org\junit\experimental\max\MaxHistory.java

METHOD NAME : testStarted(Description description) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\experimental\max\MaxHistory.java

METHOD NAME : testFinished(Description description) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\experimental\max\MaxHistory.java

METHOD NAME : testFailure(Failure failure) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\experimental\max\MaxHistory.java

METHOD NAME : testRunFinished(Result result) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\experimental\results\FailureList.java

CATCH CLAUSE : catch (Exception e) { throw new RuntimeException("I can't believe this happened"); }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\org\junit\experimental\theories\internal\Assignments.java

---

FILE NAME : JUnit\org\junit\experimental\theories\Theories.java

METHOD NAME : createTest() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\internal\builders\AnnotatedBuilder.java

METHOD NAME : runnerForClass(Class<?> testClass) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\internal\builders\AnnotatedBuilder.java

METHOD NAME : buildRunner(Class<? extends Runner> runnerClass,Class<?> testClass) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\internal\runners\JUnit4ClassRunner.java

METHOD NAME : createTest() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\internal\runners\JUnit4ClassRunner.java

CATCH CLAUSE : catch (Exception e) { testAborted(notifier,description,e); return; }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\org\junit\internal\runners\MethodRoadie.java

METHOD NAME : call() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\internal\runners\MethodRoadie.java

CATCH CLAUSE : catch (Exception e) { addFailure(e); }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\org\junit\internal\runners\MethodRoadie.java

CATCH CLAUSE : catch (Exception e) { throw new RuntimeException("test should never throw an exception to this level"); }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\org\junit\internal\runners\MethodValidator.java

CATCH CLAUSE : catch (Exception e) { fErrors.add(new Exception("Test class should have public zero-argument constructor",e)); }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\org\junit\internal\runners\statements\ExpectException.java

METHOD NAME : evaluate() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\internal\runners\statements\FailOnTimeout.java

METHOD NAME : throwTimeoutException(StatementThread thread) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\rules\ErrorCollector.java

METHOD NAME : call() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runner\Description.java

CATCH CLAUSE : catch (ClassNotFoundException e) { return null; }

ANTI-PATERN: just returns null instead of handling or re-throwing the exception, swallows the exception, losing the information forever

---

---

FILE NAME : JUnit\org\junit\runner\notification\RunListener.java

METHOD NAME : testRunStarted(Description description) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runner\notification\RunListener.java

METHOD NAME : testRunFinished(Result result) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runner\notification\RunListener.java

METHOD NAME : testStarted(Description description) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runner\notification\RunListener.java

METHOD NAME : testFinished(Description description) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runner\notification\RunListener.java

METHOD NAME : testFailure(Failure failure) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runner\notification\RunListener.java

---

FILE NAME : JUnit\org\junit\runner\notification\RunNotifier.java

CATCH CLAUSE : catch (Exception e) { failures.add(new Failure(Description.TEST_MECHANISM,e)); }

ANTI-PATTERN : catching generic Exception, catch the specific exception that can be thrown

---

---

FILE NAME : JUnit\org\junit\runner\notification\RunNotifier.java

---

FILE NAME : JUnit\org\junit\runner\Result.java

METHOD NAME : testRunStarted(Description description) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runner\Result.java

METHOD NAME : testRunFinished(Result result) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runner\Result.java

METHOD NAME : testFinished(Description description) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runner\Result.java

METHOD NAME : testFailure(Failure failure) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runner\Result.java

METHOD NAME : testIgnored(Description description) throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runners\BlockJUnit4ClassRunner.java

METHOD NAME : createTest() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runners\Parameterized.java

METHOD NAME : createTest() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runners\Parameterized.java

METHOD NAME : createTestUsingConstructorInjection() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runners\Parameterized.java

METHOD NAME : createTestUsingFieldInjection() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runners\Parameterized.java

METHOD NAME : getParametersMethod() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runners\Parameterized.java

METHOD NAME : createRunnersForParameters(Iterable<Object[]> allParameters,String namePattern) throws InitializationError, Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

---

FILE NAME : JUnit\org\junit\runners\Parameterized.java

METHOD NAME : parametersMethodReturnedWrongType() throws Exception

ANTI-PATTERN: Throws generic Exception, defeats the purpose of using a checked exception, declare the specific checked exceptions that your method can throw

---

EXPERIMENTAL RESULTS

NUMBER OF JAVA FILES :164 NUMBER OF CATCH CLAUSES :109 NUMBER OF METHOD DECLARATIONS :1095

NUMBER OF THROW PRINTSTACKTRACE ANTIPATTERN:0 NUMBER OF THROW LOG ANTIPATTERN:0 NUMBER OF CATCH ALL ANTIPATTERN:13 NUMBER OF RETURN NULL LOG ANTIPATTERN:0 NUMBER OF RETURN NULL PRINTSTACKTRACE ANTIPATTERN:0 NUMBER OF MULTI-LINE LOG ANTIPATTERN:0 NUMBER OF CATCH-AND-IGNORE ANTIPATTERN:2 NUMBER OF THROWS EXCEPTION ANTIPATTERN:39 NUMBER OF DESTRUCTIVE WRAPPING ANTIPATTERN:0 NUMBER OF RELYING ON GETCAUSE ANTIPATTERN:0

[kcooney]

All of these are false positives, except for the one in MaxCore

JUnit intentionally has method signatures that declare 'throws Exception' to make it very easy to write tests without worrying about exceptions. The framework catches them for you. The usual problem of 'throws Exception' is it adds a burden on the caller, but that isn't the case here.

JUnit needs to catch Exception to report failing tests.

Two of the errors about catching an exception and ignoring it are false positives. The code actually does report it; we don't need to throw or log the exception to handle these cases. The one in MaxCore may be a bug; feel free to send us a pull request.

[ashishsureka]

Thanks - I get the point. In-case of JUnit or test-code (different technique in unit test-code) “Throws generic Exception” and “Catching generic Exception” is not an anti-pattern. I have made a note of it and will incorporate it in my tool. I am working towards adding more Exception handling anti-patterns in my tool which I will make it free and open-source. Following are two bugs. Please confirm. Please let me know if I should create a new issue mentioning the following two anti-patterns? Thanks for your feedback.

FILE NAME : JUnit\org\junit\experimental\max\MaxCore.java CATCH CLAUSE : catch (ClassNotFoundException e) { return null; } ANTIPATTERN : just returns null instead of handling or re-throwing the exception, swallows the exception, losing the information forever

---

FILE NAME : JUnit\org\junit\runner\Description.java CATCH CLAUSE : catch (ClassNotFoundException e) { return null; } ANTIPATTERN : just returns null instead of handling or re-throwing the exception, swallows the exception, losing the information forever

[kcooney]

@ashishsureka the second one is not an error. Did you look at the code?

[ashishsureka]

I am sorry, please explain why it is not an anti-pattern. The catch clause just returns null instead of handling or re-throwing the exception. It swallows the exception losing the information forever.

[kcooney]

@ashishsureka I agree that in most situations, that is an anti-pattern, but if you look at at the second example in context, there are no problems. Not all Descriptions are associated with classes, but we cannot tell if it represents a class without trying to load the class. If there isn't a class associated with the Description the method states that it should return null, which is reasonable behavior IMHO. There's no information lost here.

[ashishsureka]

In my opinion – both are anti-patterns (Description.java and MaxCore.java). Following should be the right way. Please confirm and I will create a pull request. Thanks

Description.java

try { fTestClass = Class.forName(name, false, getClass().getClassLoader()); return fTestClass; } catch (ClassNotFoundException e) { e.printStackTrace(); return null; }

MaxCore.java

private Class<?> getMalformedTestClass(Description each) { try { return Class.forName(each.toString().replace(MALFORMED_JUNIT_3_TEST_CLASS_PREFIX, "")); } catch (ClassNotFoundException e) { e.printStackTrace(); return null; } }

[stefanbirkner]

Calling printStackTrace() is bad. Test should never write to stdin or stderr, because a test has only one outcome: it fails of it doesn't fail.

[kcooney]

We could avoid the exception entirely by doing something like this: http://stackoverflow.com/a/2036139/95725

Edit: looking further, that might not work for some class loader implementations.