I have a Master’s degree in computer science and I'm a huge Java addict. I originally worked for Tocea’s R&D team where I took part in the development of Tocea’s audit and refactoring softwares. I am now a member of the professional services team where my mission is to provide companies with solutions to tackle their technical debt. Armel has posted 7 posts at DZone. You can read more from them at their website. View Full User Profile

5 Highly Critical, Yet Rarely Used, PMD Rules

09.06.2012
| 7345 views |
  • submit to reddit

Going through the statistics of rules’ usage on Techdebt.org I was surprised to find critical PMD rules that seem to be disregarded. Here is a presentation of 5 of them, with an explanation on what they check. Those rules all have two things in common:

  • Their non-respect can bring huge problems into your application ;
  • Their usage statistic is surprisingly low.

For each rule you will also find its usage statistics (the percentage of project where the rule is used, according to Techdebt.org) and the PMD’s ruleset where you can find the rule. You may not learn anything new, but don’t forget to check your quality profile to make sure you use them ;)

 

ReturnFromFinallyBlock

Usage : 7.75% PMD ruleset : Basic

This rule finds return in finally blocks. This is definitely a critical mistake since it can discard exceptions. If an exception, checked or unchecked, is thrown in the try block, a return in the finally block will discard it.

Example : the following code only prints “Finally”.

   public static void main(final String[] args) {
        try {
            foo();
        }
        catch (final Exception e) {
            System.out.println("Catch");
        }
    }
    public static int foo() throws Exception {  
        try {
            // some clever code that throws an Exception
            throw new Exception();
        }
        finally {
            System.out.println("Finally");
            return -1;
        }
    }

 

DoNotThrowExceptionInFinally

Usage : 7.51% PMD ruleset : Strict Exceptions

Throwing an exception in a finally block can be really confusing and makes the code hard to debug. It will also discard any previous exception raised. For example, if an unexpected exception, like a NullPointerException, is thrown in the try block, it will be discarded. This can hide bugs, resulting in painful debugging tasks…

Example : the following code prints “Gotcha!” when you would like it to crash with a NPE.

    public static void main(final String[] args) {  
        try {
            foo();
        }
        catch (final IOException e) {
            System.out.println("Gotcha!");
        }
    }
    public static void foo() throws IOException {
        try {
            // some clever code...
            // unexpected exception
            throw new NullPointerException();
        }
        finally {
            // do something
            // throw IOException, the NullPointerException is discarded.
            throw new IOException();
        }
    } 

 

DontCallThreadRun

Usage : 1.08% PMD ruleset : Basic

The run() method is executed in the current thread. To create a new thread, the method start() must be used, which is usually the intended behavior. This rule is also present in Findbugs with the name “RU_INVOKE_RUN”. One of this rule should be activated.

Example : this code illustrate the behavior of thread.run() and thread.start().

    public static void main(final String[] args) {
        System.out.println("Main thread: " + Thread.currentThread().getId());
        final FooThread thread = new FooThread();
        thread.run();
        thread.start();
    }
    public static class FooThread extends Thread
    {
        @Override
        public void run() {
            System.out.println("I'm executing from thread " + Thread.currentThread().getId());
            super.run();
        }
    }

 

AvoidStringBufferField

Usage : 7.51% PMD ruleset :  String and StringBuffer

The use of a StringBuffer as a class’ field should be avoided. A StringBuffer can use a lot of memory. If the class has a long time life, this can lead to memory overflow. As long as it is possible, prefer to use local variables.

Example : don’t do that

   public class ExampleClass
   {
       private StringBuffer output;

 

FinalizeShouldBeProtected

Usage : 7.57% PMD ruleset : Finalizer

Finalize() is called by the garbage collector when an object is collected. You should not rely on finalize to perform tasks other than freeing resources. Finalize is not meant to be called by anyone other than the garbage collector. For this reason, finalize() should not be public but protected. This rule is also in Findbugs, with the name “FI_PUBLIC_SHOULD_BE_PROTECTED”. At least one of this rule should be used.

Example : again, don’t do that

    @Override
    public void finalize() throws Throwable {
        ...
    }

 

 

That’s the end of this article, as we have seen, violating these rules can bring some trouble. I was surprised to see that they are rarely used, so tell me : did you know about these rules? Do you think they’re worth checking?

Published at DZone with permission of its author, Armel Gouriou. (source)

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Tomasz O. replied on Fri, 2012/09/07 - 1:48am

Hi,

 

I didn't know that StringBuffer shouln't be a class field because of memory usage problems.

Regarding returning or throwing exceptions from finaly block, well, this is something that I would never do.. without thinking to much about it, it just doesn't look right.

 

Regards,

Tomek

Stephane Vaucher replied on Fri, 2012/09/07 - 9:34pm

Most of these are symptoms of incorrect usage of Java classes/constructs. I'm not sure about the criticality of these. Two comments:

* AvoidStringBufferField

Technically, this is a standard problem with any mutable field. I agree that I would stay away from using a StringBuffer as I mostly use it for String construction.

* Furthermore, explicitly calling finalize()doesn’t guarantee its execution

 Unsure about this statement. Why would finalize() not be executed if you call it directly? However, it is certainly weird to do this, much like calling Thread.run(). 

Armel Gouriou replied on Mon, 2012/09/17 - 1:10pm

Hi,

Thanks for your comments, sorry for the late answer!

Tomasz, you're absolutetly right, this doesn't look right. Yet it's sometimes present in code: http://techdebt.org/ tells us there is 69 violations of ReturnFromFinallyBlock and 312 violations of DoNotThrowExceptionInFinally. So if you need to perform maintainance tasks on an application developed by someone else, this can be worth checking.

Stéphane, these are indeed incorrect usages of Java. They can have important impact on the behaviour of an application, that's why I consider them as critical. And since there are rules to detect them, why not use these rules?

Regarding finalize(), you're right, I didn't get it clearly. I've clarified the point.


 

Nachiket Naik replied on Mon, 2013/10/28 - 7:51pm

Hi,

I wanted to know from where you got the usage data for each rule? Is there some resource for the same?

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.