List of Exception Management Anti-Patterns and Code Smells

[asfimport]

Ashish Sureka (Bug 56682): 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 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.

Created attachment JMETER-211-EXCEPTION-HANDLING-ANTIPATTERNS.txt: List of Exception Management Anti-Patterns and Code Smells

OS: All

[asfimport]

Sebb (migrated from Bugzilla): There's a slight code smell in the generated output :-) Most of the messages have the prefix:

ANTI-PATTERN: but some have ANTI-PATTERN :

This can make it harder to process the results, as well as indicating possible code duplication/corruption in the tool.

Also the summary totals at the end don't use the same wording as the individual messages so it's not immediately obvious which total refers to which message. This makes the summary totals harder to use.

Minor nit: the file path names use the Windows path separator () which can cause issues for users on other OSes. Windows Java can use / in paths, so it is more generally useful to use that in output that applies to multiple OSes.

Further, the output might be easier to use in a format that better suited automatic processing. For example, CSV.

==

I've not checked all the results; there are too many. I did not spot any obvious false positives, but whether all the antipatterns really represent problems is another matter. The article is quite old, and recent practice may have changed for some of the antipatterns.

[asfimport]

Sebb (migrated from Bugzilla): Also just noticed that the tool was also run on the test code. There are often good reasons to use different techniques in unit test code.

[asfimport]

Sebb (migrated from Bugzilla): I only counted 982 Java files in the report, yet the total at the end says:

NUMBER OF JAVA FILES :1053

There do not seem to be any warning messages corresponding to the following:

NUMBER OF CATCH-AND-IGNORE ANTIPATTERN:7

[asfimport]

Sebb (migrated from Bugzilla): (In reply to Sebb from comment 2)

Also just noticed that the tool was also run on the test code. There are often good reasons to use different techniques in unit test code.

In particular, JUnit3 tests normally look like

public void testXYZ() throws Exception {...}

I don't think anyone would agree that is a code smell.

[asfimport]

Sebb (migrated from Bugzilla): Just found a false-positive:

FILE NAME : apache-jmeter-2.11\src\jorphan\org\apache\jorphan\collections\HashTree.java

CATCH CLAUSE : catch (RuntimeException e) { if (!e.getMessage().equals(FOUND)) { throw e; } }

ANTI-PATTERN: Wrapping the exception and passing getMessage() destroys the stack trace of original exception

[asfimport]

Sebb (migrated from Bugzilla): The report does not have line numbers.

[asfimport]

Sebb (migrated from Bugzilla): (In reply to Sebb from comment 1)

... whether all the antipatterns really represent problems is another matter.

For example, catching Exception rather than a list of specific Exceptions.

For code that wants to be able to continue following an Exception, it would be necessary to catch all the possible checked Exceptions as well as RuntimeException. This can get very messy.

[asfimport]

Ashish Sureka (migrated from Bugzilla): Created attachment J-METER-2-11-EXCEPTION-HANDLING-ANTIPATTERNS.txt: List of Exception Management Anti-Patterns and Code Smells

[asfimport]

Ashish Sureka (migrated from Bugzilla): Thanks for the feedback – really appreciate it. I have addressed all the comments/issues mentioned in this discussion thread and fixed it (except the minor nit which also I will address soon). I have attached the new report (refer to Comment 8 and Attachments).

I agree that for the unit-test-code throwing and catching generic exception (Exception) is not an anti-pattern and I need to figure out a solution to this problem (warning message based on whether test-code or not)

I am working towards enhancing the tool to fix issues and add more exception handling anti-patterns (currently it has 10 anti-patterns). I will make the tool free and open-source and host it on GitHub. Thanks

[asfimport]

Sebb (migrated from Bugzilla): (In reply to Ashish Sureka from comment 9)

Thanks for the feedback – really appreciate it. I have addressed all the comments/issues mentioned in this discussion thread and fixed it (except the minor nit which also I will address soon). I have attached the new report (refer to Comment 8 and Attachments).

I was referring to line numbers for the source code.

I agree that for the unit-test-code throwing and catching generic exception (Exception) is not an anti-pattern and I need to figure out a solution to this problem (warning message based on whether test-code or not)

There are likely other anti-patterns that are common in Unit test code.

It would probably be better to produce separate reports for the two source trees.

I am working towards enhancing the tool to fix issues and add more exception handling anti-patterns (currently it has 10 anti-patterns). I will make the tool free and open-source and host it on GitHub. Thanks

Note that there are already a lot of other such tools. For example, Findbugs.

[asfimport]

Ashish Sureka (migrated from Bugzilla): Sebb: I was referring to line numbers for the source code.

Ashish: Mentioning line-number of the source code and inserting line-numbers in the report – both are useful. I will work towards adding source-code line numbers also (good feature).

Sebb: There are likely other anti-patterns that are common in Unit test code. It would probably be better to produce separate reports for the two source trees.

Ashish: I agree. This will be a good feature. However, I need to come-up with a solution to differentiate between test-code and non-test-code.

Sebb: Note that there are already a lot of other such tools. For example, Findbugs.

Ashish: The focus of my tool is Exception handling anti-patterns and code-smells. While there are several anti-pattern detecting tools, they do not cover exception handling anti-patterns. Here’s the link to FindBugs Bug Descriptions and it does not cover the anti-patterns in my tool (+ I am implementing more anti-patterns which are not covered in FindBugs).

Thanks again for the inputs. FYI – I am submitting issue-reports to some more open-source Java projects and I a observing that some anti-patterns from the report are accepted as issues and assigned for fix.

[asfimport]

@pmouawad (migrated from Bugzilla): Hello, It would be nice if tool could generate a CSV with those informations.

Thanks

[asfimport]

@pmouawad (migrated from Bugzilla): Closing as current report is not usable. Many reported issues are not really ones, some have been fixed, output is hard to use and test code is mixed with non test code.