I have
if (localName.equals("TaxName")) {
but PMD says
Position literals first in String comparisons
views:
121answers:
4
+6
A:
"TaxName".equals(localName) is better as if localName is null you won't get a null pointer exception.
Adam
2010-06-23 20:44:59
+3
A:
I prefer to position literals first, i.e. :
if ("TaxName".equals(localName)) { ...
This way you do a right comparison for the case of null, instead of getting NullPointerException.
Eyal Schneider
2010-06-23 20:46:00
+4
A:
PMD should also be telling you why it generates this warning. From the rules documentation on the PMD website:
Position literals first in String comparisons - that way if the String is null you won't get a NullPointerException, it'll just return false.
matt b
2010-06-23 20:46:11
+1
A:
Personally, that doesn't make sense to me. If the code catches a NullPointerException, then it's done work that you won't have to do later. If localName ends up being null, and that causes a problem later on, then it'll be harder to trace. Don't change code to make the compiler happy. If your code throws a NullPointerException, then it's saved you debugging time later.
Matt
2010-06-23 20:50:39
I totally disagree and definitely prefer to write `CONSTANT.equals(variable)` rather than doing a preliminary `null` check.
Pascal Thivent
2010-06-23 21:01:07
If localName is null and you try to use it later on you'll get exactly the same effect as the original code, i.e an NPE. But, more significantly, one should always make the compiler happy - it knows what its doing. I would even recommend going into the compiler settings for your IDE of choice and upgrade all those compiler warnings into errors.
CurtainDog
2010-06-23 21:05:54
@CurtainDog upgrading compiler warnings into errors would effectively stall my company's productivity for a year. I'm sure 90% of the people on SO would agree. I'm not saying you're not right in principle, because those warnings turn into bugs all too often, I'm just saying in it's easier said than done.
glowcoder
2010-06-23 21:17:20