#228 ConfusingTernary with null

[ereissner]

Hello everyone,

this is maybe more a changerequest than a bug-report.

i think

if (x != null) {
...
} else {
...
}

conforms with the rule of thumb to handle the common
case first
and then the exceptional one.
Nevertheless this is flagged by pmd.

So i suggest to flag

if (x != y) {
...
} else {
...
}

only if neither x nor y is litterally "null".

greetings

Ernst

[Tom Copeland]

Logged In: YES
user_id=5159

Hm, it's an interesting thought. I've asked the rule's
author, Todd Wright, what he thinks about it...

Yours,

tom

[rei3ner]

Ok, and what did He say?

[Tom Copeland]

Logged In: YES
user_id=5159

I think we'll leave this one as it is - good discussion in
the emails, Ernst. Of course, you can certainly roll your
own variation of the rule.

Cool stuff!

Yours,

Tom

[rei3ner]

Well, Tom, i do not want to write my own rule.
I would like to stick to generally accepted standard rules,
since i also write in standard java and not in my own language.

I have another thought.

a!=null is equivalent with a instanceof Object.
One could also write a method isObject(Object)
checking whether the argument links to an object.
If not, it is just null.

Thus the a!=null branch shall be the then branch of the if-statement.

What do you think of the idea, to offer two flavors of this rule?
and maybe both as controversial?

[Anthony Whitford]

Logged In: YES
user_id=1344312

Yes, I find this rule is simply crying wolf because if-then
statements are written such that the nominal path is coded
first, then the unusual case. (See Code Complete, Second
Edition, chapter 15.1, page 355.)

Please consider reclassifying this rule as controversial,
rather than a design rule.

[rei3ner]

I heared that some people just disable this rule because they want
no failure for if(a!=null).
This isunfortunate because the rule as such is very valuable.

[Tom Copeland]

Logged In: YES
user_id=5159

Hm, reopening this for reconsideration...

tom

[Jason Bennett]

Logged In: YES
user_id=61134

This rule might also want to consider else if's as negating
the rule.

That is, if I have:

if (x != null) {
do y;
} else if (z != null) {
do a;
}

This structure is not easily changed according to this rule.
Usually this structure wants to choose among several
assignment/return possibilities based on which variables are
null and which are not.