Problems with AvoidDeeplyNestedIfStmts
A source code analyzer
Brought to you by:
adangel,
juansotuyo
The rule AvoidDeeplyNestedIfStmts says this is against
the rule, but this isn't nested statements. I am not
saying this is good practise, but maybe some other rule
should handle this case.
This may be a factory method, and then this is the only
option.
if( i == 20 ) {
//something
}
else if( i == 21 ){
//something else
}
else if( i == 503 ) {
// and something else
}
Logged In: YES
user_id=5159
The problem is that the AST for those if stmts has all the else
clauses nested inside each other. When I first wrote the rule
I wasn't sure how to get that right.... I'll look at it again.
Thanks,
Tom
Logged In: YES
user_id=394445
ok, so the AST generates something like this?
if(...) {
}
else {
if( ... ) {
}
else {
if(...) {
}
}
}
Logged In: YES
user_id=5159
You can tell I was in denial on this one:
http://pmd.sourceforge.net/xref/test/net/sourceforge/pmd/rules
/AvoidDeeplyNestedIfStmtsRuleTest.html
Logged In: YES
user_id=5159
Right, so this code:
public class AvoidDeeplyNestedIfStmtsRule2 {
public void bar() {
if (true) {
} else if (true) {
} else if (true) {
} else {
// this ain't good code, but it shouldn't trigger this rule
}
}
}
produces this AST (minus some irrelevant stuff)
You can see how the AST is nested:
IfStmt
Stmt
IfStmt
Stmt
IfStmt
even though those are actually else blocks, not if blocks.
Maybe we can add some stuff to the grammer to differentiate
if blocks from else blocks....
Tom
Logged In: YES
user_id=5159
By the way, a good way to view the AST is to run the
astviewer utility. Just run that astviewer.bat batch file in the
etc directory and type or paste some code into the left hand
window and click the button. The AST will be displayed in
the right hand pane.
Tom
Logged In: YES
user_id=5159
Fixed. Required a tweak to the grammer IfStatement
production and a minor twiddle of the rule.
Thanks,
Tom