Posted by Armel GOURIOU in Coding Rules and Guidelines on September 3, 2012 with Comments Off
Going through the statistics of rules’ usage on Techdebt.org I was surprised to find critical PMD rules that seems 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:
For each rule you will also find its usage percentage, the PMD’s ruleset where you can find it and the criticity recommended by Tocea. You may not learn anything new, but at least don’t forget to check your quality profile to make sur you use them
| Criticity : Blocker | 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”.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
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;
}
} |
| Criticity : Blocker | 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.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 |
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();
}
} |
| Criticity : Blocker | 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().
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 |
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();
}
} |
| Criticity : Blocker | 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
|
1 2 3 |
public class ExampleClass
{
private StringBuffer output; |
| Criticity : Blocker | 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
|
1 2 3 4 |
@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?
Comments are closed.