Same same - but differentEnum values are guaranteed singletons - and thus the == reference comparison is safe. The enum itself just delegates to Object.equals(), which does exactly that.
So if they execute the same code, why should I bother?
Learning the hard way - actual bugI produced a bug which broke the build. Luckily we did have a unit test in place that caught it. But still I would have seen it earlier, had I used the "correct" syntax.
So which one is it?
And the winner is: ==It provides compile-time safety. While MyColor.BLACK.equals(acceptsAnything)gives a warning at most, the check MyColor.BLACK == something won't even compile if something is not an instance of MyColor.
This makes it refactoring-safe. If you go - like me - and change the type of something later on, you'll notice instantly.
Another difference is the support for null. The check with == allows null on both sides. It doesn't throw, however, it may also "hide" a null value where there shouldn't be one.
The check with equals() allows null only on the right side, and throws a NullPointerException if the left side is null. That may be desired sometimes. Anyway, I vote for not letting null slip into this comparison at all. It should be checked/handled before, instead of being smart. (See my article about null handling.)
So the comparison with == is "safer" also at run-time in that it never throws a NPE, but is possibly hiding a bug when the left side should never have been null.
Then there's the argument of performance. It's irrelevant. I'm not going there.
And the visual aspect. Which one looks nicer?
When I can have compile-time safety, I don't care about the looks. I was used to .equals() simply because that's how Strings are compared. But in retrospect that's a pretty lame explanation.
On this StackOverflow question, which is exactly about the topic, Kevin Bourrillion from Guava commented that "== may appear incorrect to the reader until he looks at the types" and concludes that "In that sense, it's less distracting to read ".equals()". Au contraire! When I see CONSTANTS around == I instantly know they're either enums, or primitives, of matching types. If not, the code is either red because it doesn't compile, or with a yellow warning from my intelligent IDE saying that .equals() should be used. For example Strings or primitive wrappers like Integer.
After that incident at the company we've decided to go with reference equality comparison ==.
== breaks type abstraction where equals() does not.ReplyDelete
In other words your implementation is now married to your interface and all of those =='s will break the moment you need to define a super interface.
See: "Emulate extensible enums with interfaces" Effective Java 2nd Edition. Bloch. p 145.
Hello Fabian nice post, If you enum is not extensible then == is better option but as Karl suggested it might not work expected when used with Enum as Type, by the way I have also shared my thoughts on Comparing two Enums in Java. Let me know how do you find it.ReplyDelete
Enums are guaranteed singletons, except when they aren't. I know of at least one - very nasty - way that you can get two *different* Enum instances that represent the same Enum value, and when that happens then == doesn't do the right thing. (Or it does do the right thing, depending on your point of view, but your code still doesn't work)ReplyDelete
That way is if the enum value is un/marshalled over a CORBA interface. When it comes back in to your code, the CORBA interface creates a new instance of the Enum class rather than using the existing singleton instance.
Yes, I hit this in a real product I was working on, and yes it was a nightmare to work out what was happening...
And, of course, if you aren't using anything like CORBA in your app then I'd go right ahead and use == because it works :)
Think about if someone change Color from Enum to class. == will also break the logic without complie time error. So in my opinion == isn't a good solution for everything. In your case, it is a bug due to partial/incomplete refactoring, which is indeed hard to detect.ReplyDelete
To complete your refactor, I think you can change Color to a "constant holding class"
public static final RED = new RgbColor(0, 0, 255);
public static final YELLOW = ...
public static final BLUE = ...
Then equals() works like a charm.