views:

514

answers:

4

Is there a good resource which describes the "why" behind PMD rule sets? PMD's site has the "what" - what each rule does - but it doesn't describe why PMD has that rule and why ignoring that rule can get you in trouble in the real world. In particular, I'm interested in knowing why PMD has the AvoidInstantiatingObjectsInLoops and OnlyOneReturn rules (the first seems necessary if you need to create a new object corresponding to each object in a collection, the second seems like it is a necessity in many cases that return a value based on some criteria), but what I'm really after is a link somewhere describing the "why" behind a majority of PMD's rules, since this comes up often enough.

Just to be clear, I know that I can disable these and how to do that, I'm just wondering why they are there in the first place. Sorry if there's something obvious I missed out there, but I did a Google search and SO search before posting this. I also understand that these issues are often a matter of "taste" - what I'm looking for is what the argument for the rules are and what alternatives there are. To give a concrete example, how are you supposed to implement one object corresponding to every object in a loop (which is a common operation in Java) without instantiating each object in a loop?

+6  A: 

In each case, the rule can be a matter of specific circumstances or just "taste".

Instantiating an Object in a loop should be avoided if there are a large number of iterations and the instantiation is expensive. If you can move the code out of the loop, you will avoid many object instantiations, and therefore improve performance. Having said that, this isn't always possible, and in some cases it just doesn't matter to the overall performance of the code... in these cases, just do what is clearer.

For OnlyOneReturn, see the Stack Overflow question on the same subject. There are several ways to view this (with vehement supporters behind each), but they all basically boil down to taste.

For your example, the OnlyOneReturn people want code such as:

public int performAction(String input) {
    int result;
    if (input.equals("bob")) {
        result = 1;
    } else {
        result = 2;
    }
    return result;
}

Rather than:

public int performAction(String input) {
    if (input.equals("bob")) {
        return 1;
    } else {
        return 2;
    }
}

As you can see, the additional clarity of ReturnOnlyOnce can be debated.

There's a similar question relating to instantiation within loops as well.

jsight
+1 PMD shouldn't most of the time be taken very seriously compared to FindBugs for example.
ponzao
the last sample should generate an "unnecessary else clause" warning anyway :)
matt b
@matt b - That is also a matter of taste. :)
jsight
A: 

You can look at the PMD-homepage, the rules are explained here in detail and often with a why. The site is structured for the rules-groups, here the link to basic-rules: http://pmd.sourceforge.net/rules/basic.html

Dishayloo
As I said in my post, I know about this, this isn't what I'm looking for, because most of the time there is no why (specifically for the two I'm looking for).
James Kingsbery
A: 

Each rule is in a PMD Rule Set, which can give you a clue to the reasoning behind the rule (if it isn't explained in detail on the Rule Set page itself).

In the case of AvoidInstantiatingObjectsInLoops, it can be expensive to instantiate a similar object again and again. However it is frequently necessary. On my own project, I have disable this rule, since it is flagging too many false positives.

In the case of OnlyOneReturn, note that it is in a Rule Set called Controversial, which is a hint that these rules are debatable, and depend on the case. I have disabled this entire Rule Set as well.

Avi
Yeah, like I said I know that you can, and I often do, disable rules.
James Kingsbery
+1  A: 

This article, A Comparison of Bug Finding Tools for Java, "by Nick Rutar, Christian Almazan, and Jeff Foster, compares several bug checkers for Java..."—FindBugs Documents and Publications. PMD is seen to be rather more verbose.

Addendum: As the authors suggest,

"all of the tools choose different tradeoffs between generating false positives and false negatives."

In particular, AvoidInstantiatingObjectsInLoops may not be a bug at all if that is the intent. It's included to help Avoid creating unnecessary objects. Likewise OnlyOneReturn is suggestive in nature. Multiple returns represent a form of goto, sometimes considered harmful, but reasonably used to improve readability.

My pet peeve is people who mandate the use of such tools without understanding the notion of false positives.

trashgod
Interesting paper, but not what I was asking. As I said above, I know I can disable these warnings, what I'm wondering is what the reasoning behind PMD providing these warnings.
James Kingsbery
Ah, you wanted elucidation of those two rules in particular. I've elaborated above, but the answer cited by @jsight addresses _OnlyOneReturn_ particularly well.
trashgod
I had never really thought of return as being akin to goto, that's helpful.
James Kingsbery
I take them for granted now, but several useful flow-of-control mechanisms may be considered "deviations" from structured programming. http://en.wikipedia.org/wiki/Structured_programming#Common_deviations
trashgod