== equals()

I saw this in a code example somewhere earlier this morning and after thinking about it for a while decided to write a short post on it.

if (stringVar1 == stringVar2 || stringVar1.equals(stringVar2)) {

I've seen this other places, by other developers.  Discussing this is interesting, because depending on your level of expertise, you might have many reactions to this code snippet:

The inexperienced Java coder will wonder why you'd use the .equals() at all ...

Someone with at least a little experience knows that two String variables can be different and yet represent the same sequence of characters, which is the purpose of .equals().  That programmer might be thoroughly confused as to why you'd ever use == when it's not guaranteed to work.

A slightly more experienced programmer will probably assume that the == is there as a performance tweak, to avoid the overhead of the call to .equals() if the two variables happen to point to the same String object.

But the really experienced programmer will note that the whole thing is just stupid.  Why?  Because the first thing that String.equals() does is test to see if the two variables point to the same object, so the alleged performance tweak is insignificant.  Additionally, since the actual chance that both of those variables point to the same object is very slim in most use cases, this construct is probably making the code slower most of the time.

Programmers should know this.  Beyond simply the String implementation, the Java documentation specifically recommends that the first thing that an equals() implementation should do is test for the same object reference.  It's widely accepted best practice ... and most IDEs will generate that code for you if you right-click and then hit whatever button automatically generates equals() and hashcode().  So why do people still do this?  I can't really be sure.

This isn't a terribly common construct, but it's common enough that it makes me worry about how much young coders "code by example" and how dangerous it is that there are so many bad examples out there to copy from (despite the number of good examples also extant).

The reason it stuck in my head is that shortly after seeing it, I saw another bad code example in the Android API docs.  The docs for the Android TestCase class recommend that you use assertTrue(result == 5.0) instead of using assertEquals(5.0, result).  Of course, anyone who's done much with unit testing knows that the former will provide a cryptic, unhelpful message in the event of a failure; whereas the latter will at least tell you what was expected and what was actually achieved, which starts you down the road to solving the problem.  What's an even greater irony in this case is that the example is supposed to be of assertTrue(String, boolean), thus hammering home the point that unit test failures should be verbose enough to be helpful.