« lmbench results for Amazon EC2 | Main | The pain and joy of moving from MovableType-3.14 to 3.33 »

How hard is test for equality in Java -- the answer will surprise you

A post at THE DAILY WTF includes a function with a compound if statement consisting of eight lines of code to set a flag when the Boolean argument is not equal to a Boolean member, suggesting that the code is correct but overly complex and verbose.

/** Set the value of the isRecordLocked attribute. @param newValue is the new isRecordLocked value. */ public void setIsRecordLocked(Boolean newValue) { // Update state if required. if (isRecordLocked != null) { if (newValue == null || !isRecordLocked.equals(newValue)) { context.setDirty(true); } } else if (newValue != null) { context.setDirty(true); } // Change the value. isRecordLocked = newValue; }

The post also includes a suggestion that the compound if statement can be replaced entirely by a much more understandable one-liner, but doesn't really include the one-liner (perhaps obvious to the submitter).

I struggled for few minutes to figure out the obvious one liner, but couldn't come up with anything signficantly better. The problem is that a simple (A != B) doesn't work for Java Objects (and Boolean is an Object), for either A or B could be null and invoking equals method on null would result in a NullPointerException.

I clicked to see the comments, hoping to find the elusive one-liner in the copious comments (137, when I checked). No luck there. Though some people pointed out that there was nothing wrong with the code, most attempts to better the code resulted in buggy code.

Update (4:40PM, Nov. 30): Got a comment via email(My comments link is disabled due to heavy comment spam and I haven't gotten around to upgrade my blogging software to include an effective filter)


I posted this comment to your blog about the thedailywtf code - your
blog software allowed me to preview, and post, then said something
along the lines of "this blog is not accepting comments"..



Because context.setDirty should only be invoked conditionally, and the
guess is that it's a void method, the ternary operator is of no use.
The best I can come up with is:

if (isRecordLocked==newValue || (isRecordLocked != null &&

It might be possible to sandwich the last line into the if statement
somewhere, but I can't see how to do that while guaranteeing its
execution, and preserving the if statement's results.

If invoking context.setDirty(false) when the condition failed was
acceptable (unlikely), then this would work, still 2 lines though:

context.setDirty(isRecordLocked==newValue || (isRecordLocked != null
&& isRecordLocked.equals(newValue));

Actually, this replacement is problematic (read, buggy!): it attempts to call context.setDirty(true) when both the member (isRecordLocked) and the argument (newValue)are equal, whereas the original code did it the other way round. Notice the emphasis on attempts -- the attempt fails when isRecordLocked is null and newValue is not ( both isRecordLocked==newValue and isRecordLocked != null &&
will evaluate to false).

Besides, I don't think the replacement is any better in terms of overall readability!

In any case, it just proves my original point -- test of equality, or rather inequality, of two objects is pretty hard in Java.

Update (9:50PM, Dec. 1): Got the following comment, again via email, from the previous commenter Ricky:

You're right, my code was naive. I should probably have written a test that passed with the 'WTF' code, and written my implementation against that. I most likely would have done if it wasn't just for a blog. ;)

"In any case, it just proves my original point -- test of equality, or
rather inequality, of two objects is pretty hard in Java."

Actually, no. Testing equality of objects when all you have are two
references that *may* point at objects is the problem. If you
systematically disallow null (that doesn't necessarily mean testing
for it everywhere), then you're really comparing objects, not just
references, and then you've got this into a 3 liner.

if (!a.equals(b))

Yes, the problem does become much simpler if you can ensure non-null values for both the member field and the argument for all invocations of the method. This is easy to do for a private method or when it is used by only code that you or your team writes. But not if the class goes out as part of a library to be used by other developers.

Btw, I did come across a somewhat cleaner solution at behrangsa's page.

Comments (3)

Ulrich Hobelmann:

Why you don't simplify the code. First of all, why not get rid of that Boolean? The whole point of a boolean is to be either true or false, so if you're afraid that a caller could be so stupid and call with a null-Boolean, use booleans (values, not objects) instead. Any call that passes a Boolean should convert to the value automatically (or at least it's just one function call away).

The ONLY reason you should EVER use Booleans, or Integers explicitly, is for use inside a container object, such as a list, that can't hold basic values. It's sad that Java doesn't make the optimization/conversion itself, but that's how it is. If you want simple code, always use the values, not the objects. Since version 5, Java will autobox your values when it needs to.

Hobel Ulrichmann:

Containers are not the only reason to have value objects. A reference to a Boolean allows you to distinguish between having a value or not (similiar to a database NULL field).

Ulrich Hobelmann:

Oh, very funny that mixing up my name thing. You Sir must be a hyper-intelligent genius.

Sure, it might make sense to have Null values in your model somewhere, but obviously it's a very bad idea to use such values in a context like the above, that asks for a definite value because it wants to compare it to other values.

Post a comment

(If you haven't left a comment here before, you may need to be approved by the site owner before your comment will appear. Until then, it won't appear on the entry. Thanks for waiting.)


This page contains a single entry from the blog posted on November 29, 2006 3:39 PM.

The previous post in this blog was lmbench results for Amazon EC2.

The next post in this blog is The pain and joy of moving from MovableType-3.14 to 3.33.

Many more can be found on the main index page or by looking through the archives.

Powered by
Movable Type 3.33