What is the reason for these PMD rules?
DataflowAnomalyAnalysis: Found 'DD'-anomaly for variable 'variable' (lines 'n1'-'n2').
DataflowAnomalyAnalysis: Found 'DU'-anomaly for variable 'variable' (lines 'n1'-'n2').
DD and DU sound familiar...I want to say in things like testing and analysis relating to weakest pre and post conditions, but I don't remember the specifics.
NullAssignment: Assigning an Object to null is a code smell. Consider refactoring.
Wouldn't setting an object to null
assist in garbage collection, if the object is a local object (not used outside of the method)? Or is that a myth?
MethodArgumentCouldBeFinal: Parameter 'param' is not assigned and could be declared final
LocalVariableCouldBeFinal: Local variable 'variable' could be declared final
Are there any advantages to using final
parameters and variables?
LooseCoupling: Avoid using implementation types like 'LinkedList'; use the interface instead
If I know that I specifically need a LinkedList
, why would I not use one to make my intentions explicitly clear to future developers? It's one thing to return the class that's highest up the class path that makes sense, but why would I not declare my variables to be of the strictest sense?
AvoidSynchronizedAtMethodLevel: Use block level rather than method level synchronization
What advantages does block-level synchronization have over method-level synchronization?
AvoidUsingShortType: Do not use the short type
My first languages were C and C++, but in the Java world, why should I not use the type that best describes my data?
Solution 1:
DD and DU anomalies (if I remember correctly—I use FindBugs and the messages are a little different) refer to assigning a value to a local variable that is never read, usually because it is reassigned another value before ever being read. A typical case would be initializing some variable with
null
when it is declared. Don't declare the variable until it's needed.Assigning
null
to a local variable in order to "assist" the garbage collector is a myth. PMD is letting you know this is just counter-productive clutter.Specifying final on a local variable should be very useful to an optimizer, but I don't have any concrete examples of current JITs taking advantage of this hint. I have found it useful in reasoning about the correctness of my own code.
Specifying interfaces in terms of… well, interfaces is a great design practice. You can easily change implementations of the collection without impacting the caller at all. That's what interfaces are all about.
I can't think of many cases where a caller would require a
LinkedList
, since it doesn't expose any API that isn't declared by some interface. If the client relies on that API, it's available through the correct interface.Block level synchronization allows the critical section to be smaller, which allows as much work to be done concurrently as possible. Perhaps more importantly, it allows the use of a lock object that is privately controlled by the enclosing object. This way, you can guarantee that no deadlock can occur. Using the instance itself as a lock, anyone can synchronize on it incorrectly, causing deadlock.
Operands of type
short
are promoted toint
in any operations. This rule is letting you know that this promotion is occurring, and you might as well use anint
. However, using theshort
type can save memory, so if it is an instance member, I'd probably ignore that rule.
Solution 2:
DataflowAnomalyAnalysis: Found 'DD'-anomaly for variable 'variable' (lines 'n1'-'n2').
DataflowAnomalyAnalysis: Found 'DU'-anomaly for variable 'variable' (lines 'n1'-'n2').
No idea.
NullAssignment: Assigning an Object to null is a code smell. Consider refactoring.
Wouldn't setting an object to
null
assist in garbage collection, if the object is a local object (not used outside of the method)? Or is that a myth?
Objects in local methods are marked to be garbage collected once the method returns. Setting them to null won't do any difference.
Since it would make less experience developers what is that null assignment all about it may be considered a code smell.
MethodArgumentCouldBeFinal: Parameter 'param' is not assigned and could be declared final
LocalVariableCouldBeFinal: Local variable 'variable' could be declared final
Are there any advantages to using
final
parameters and variables?
It make clearer that the value won't change during the lifecycle of the object.
Also, if by any chance someone try to assign a value, the compiler will prevent this coding error at compile type.
consider this:
public void businessRule( SomeImportantArgument important ) {
if( important.xyz() ){
doXyz();
}
// some fuzzy logic here
important = new NotSoImportant();
// add for/if's/while etc
if( important.abc() ){ // <-- bug
burnTheHouse();
}
}
Suppose that you're assigned to solve some mysterious bug that from time to time burns the house.
You know what wast the parameter used, what you don't understand is WHY the burnTHeHouse
method is invoked if the conditions are not met ( according to your findings )
It make take you a while to findout that at some point in the middle, somone change the reference, and that you are using other object.
Using final
help to prevent this kind of things.
LooseCoupling: Avoid using implementation types like 'LinkedList'; use the interface instead
If I know that I specifically need a
LinkedList
, why would I not use one to make my intentions explicitly clear to future developers? It's one thing to return the class that's highest up the class path that makes sense, but why would I not declare my variables to be of the strictest sense?
There is no difference, in this case. I would think that since you are not using LinkedList
specific functionality the suggestion is fair.
Today, LinkedList could make sense, but by using an interface you help your self ( or others ) to change it easily when it wont.
For small, personal projects this may not make sense at all, but since you're using an analyzer already, I guess you care about the code quality already.
Also, helps less experienced developer to create good habits. [ I'm not saying you're one but the analyzer does not know you ;) ]
AvoidSynchronizedAtMethodLevel: Use block level rather than method level synchronization
What advantages does block-level synchronization have over method-level synchronization?
The smaller the synchronized section the better. That's it.
Also, if you synchronize at the method level you'll block the whole object. When you synchronize at block level, you just synchronize that specific section, in some situations that's what you need.
AvoidUsingShortType: Do not use the short type
My first languages were C and C++, but in the Java world, why should I not use the type that best describes my data?
I've never heard of this, and I agree with you :) I've never use short though.
My guess is that by not using it, you'll been helping your self to upgrade to int
seamlessly.
Code smells are more oriented to code quality than performance optimizations. So the advice are given for less experienced programmers and to avoid pitfalls, than to improve program speed.
This way, you could save a lot of time and frustrations when trying to change the code to fit a better design.
If it the advise doesn't make sense, just ignore them, remember, you are the developer at charge, and the tool is just that a tool. If something goes wrong, you can't blame the tool, right?
Solution 3:
Just a note on the final
question.
Putting "final" on a variable results in it only be assignable once. This does not necessarily mean that it is easier to write, but it most certainly means that it is easier to read for a future maintainer.
Please consider these points:
- any variable with a
final
can be immediately classified in "will not change value while watching". - by implication it means that if all variables which will not change are marked with final, then the variables NOT marked with final actually WILL change.
This means that you can see already when reading through the definition part which variables to look out for, as they may change value during the code, and the maintainer can spend his/her efforts better as the code is more readable.