New Java rule: use == for enum equality

289 views
Skip to first unread message

Jens Bannmann

unread,
Feb 15, 2018, 2:04:00 AM2/15/18
to SonarQube
Hi all,

I would like to propose a new Java rule which complains when enum values are compared using .equals() instead of ==.

While calling .equals() is perfectly valid, and even preferred by some for consistency with other objects, one can argue that == is superior due to type checking at compile time (e.g. this JavaWorld article by Dustin Marx).

Mostly, this is a matter of opinion, but if you have one on this matter, it is hard to enforce it across a code base without a rule in SonarQube. I would very much like to have such a rule in my company's SonarCloud quality profile.

One could also make a configurable rule that can enforce either == or .equals(), or even make two rules.

What do you all think?

Best regards,
Jens Bannmann

P.S.: Below is a message from this group that touched on the issue. At the time, the rule proposed was about .ordinal().


Michel Pawlak (2015-09-30)
Hi Michal,

Isn't a developer's first mission to write working, readable, tested and maintainable code ? Optimizing code come later.

Let me quote some some text from the post you mention about "==" usage:

enums are Objects. For all other Object types the standard comparison is .equals(), not ==

This one is really important and is the core reason why equals should be used.

It works: Until you need to replace an enum with a class as part of a refactoring. Good luck finding all the code using == that breaks.

Indeed it works, Enums equality can be verified using ==, by the way that's what is done under the hood by all (most?) VMs, but it is done for optimization reasons not for readability / maintainability reasons, and it is hidden to the enduser.
 
It's faster: Even if true (questionable), there's that Knuth quote about premature optimization.
 
Same as previous comment. Such low level optimizations should be left to the compiler + JIT compiler. And seriously "how much" faster ? with a -client or -server ? for how many hits ? And I would like to ask people doing such low level optimizations if they are sure that all their algorithms are so perfect they need to do such kind of optimization (I'm pretty sure JIT does them as well anyway)

Safer at run-time: "Safe" is not the first word that springs to mind if the use of == is the only thing separating you from a NullPointerException.

Agree, and SQ can help you write correctly the equals statement to make sure that NPE will never be thrown. You can also use @NonNull which makes the code more readable and understandable (and thus maintainable, ... and more secure in the end)

Safer at compile time: Not really. It can guard you from the rather elementary mistake of comparing the wrong types, but again, if that's how dumb your programmers are, "safe" is the one thing you're not.

I Agree (at least partially... it's not about being dumb but about making stupid mistakes anyway, but well... sleepy people can also use = instead of == in other situations)

All in all, yes, you can check enums' equality with == but it is not worth the impact on readability and maintainability.

In other words I would not change your example @Ann.

Michel

Alexandre Gigleux

unread,
Mar 28, 2018, 9:44:32 AM3/28/18
to SonarQube
Hello Jens,

It looks like a good idea to enforce '==' for enums. Here is the related specification https://jira.sonarsource.com/browse/RSPEC-4551.

Can you have a look and provide some feedback?

Thanks
Alex

Jens Bannmann

unread,
Mar 29, 2018, 3:37:11 AM3/29/18
to SonarQube
Hi Alexandre,

yes, that specification looks great and completely covers my use case.

Some minor mistakes in the text:
  • "every Java developer know" should be "every Java developer knows"
  • "it is more" and "it provides": remove the "it" so the bullet point items fit in the sentence preceding the list
  • "it is more null-safe than equals()": references to the method should be made consistent throughout the text (with or without parentheses, with or without monotype font)
  • "Noncompliant: always return false" should be "Noncompliant: always returns false"
  • "Compliant; this is only one instance" should be "Compliant; there is only one instance"
Thanks a lot for accepting my suggestion!

Best regards,
Jens

alexandr...@gmail.com

unread,
May 1, 2018, 7:24:20 AM5/1/18
to SonarQube
Thanks for the review, I fixed the typos. The implementation ticket is: https://jira.sonarsource.com/browse/SONARJAVA-2712

Regards
Reply all
Reply to author
Forward
0 new messages