Having done several contracts for different NZ Government departments, I can make a reasonable guess of the companies involved. It's probably one of these:

Inland Revenue/EDS

Accident Compensation Corporation/Unisys

My experiences when working at government department clients have always been consistent when dealing with their outsourced IT partners - they've been atrocious. Five days to get a log on, one day to get a password reset (what am I supposed to do in the meantime?). Unisys is worse - we had a Java enterprisey project and the Unisys Java Team Leader (Australian) was supposed to give us the EJB to work from - but went skiing instead, then was caught out in several lies trying to cover it up.

Is this right? I thought boolean short-circuiting will mean this is totally different from the original. For clarity (not brevity) I would be tempted to transfer the nulls to false's via a temporary to make the logic more obvious. Yes it's a waste.

I thought maybe, just maybe, New Zealand might be some kind of safe haven from WTFs (this is the first one I have seen on here). *sigh* I could see it being IRD, or maybe studylink. All our government has done with new technology is move the queues from physical locations to phones and the internet. It still takes weeks to get ANYTHING done. Knowledge wave my ass.

Actually I just figured, that that's wrong too because in the case of isLocked==false and newValue==null, which would result in setDirty(true), my new code wouldn't work.

Unless I'm missing something fundamental there is no easy way to do this to get the same answer as the original unless Boolean.valueOf() was used to create the booleans. In which case the newValue == isLocked can be used because all the TRUE/FALSE values will be the same object. I'm actually starting to prefer the WTF solution ...

I think the original WTF is fairly good code. All the the versions suggested in above comments a wrong in some way or other. Using Boolean instead of Java's built in type boolean creates this kind of hoop to jump through.

There are advantages of using Boolean, for example you can store them in data structures that take generic "Object". Or they use it just for the enterpriseyness and to please some OO fanatic. I wonder if they use Integer instead of int too...

Which won't work for two reasons. First is that you should not reset the dirty flag if it is already set to true. The second is that isRecordLocked is always different from newValue if one of them is generated by using "new Boolean(whatever)".

The real problem with the code is actually not the incomprehensive code, but the three value logic of it. Assuming here that there is no real semantic of a Boolean null, the programmer should assume that null is not possible, and use aspect oriented programming or some plain assert statements in the beginning of the function:

Umm, no. The assumption is that these are objects and that you need to call .equals to find out if the content is the same, instead of != which just checks if it's the same object. You might also be unable to pass a NULL object to .equals().

If that is so, then the original code is in fact correct. An equivalent one-liner test might be

Maybe it's because I just woke up, but I cannot get the WTF in this. Ok, it takes a while to grasp but I see nothing wrong. If the new value is different from the old value, setDirty(true). So? But I would like to see that one-liner.

But there's not much wrong with the original. I much prefer the static Object.equals that someone proposed, given that that doesn't exist, you need to have it in a library in your project somewhere. It's the only readable way.

The WTF is that it looks wrong, but when you get down to it I reckon it's right. It's just one of those times where you have to step away from the code and let someone smarter than you fiddle with it.

As far as I can see, it checks that neither of the Boolean objects are null before comparing the values and also comparing the 'nullness' too. I wouldn't like to fiddle with this until I knew what context.setDirty(...) did.

Can it be true? The original code is better than most of the comments!

This is the funniest in days - everyone trying to be smart and do it better, and almost every one trying to post a better solution getting it wrong. The original code is actually the best attempt I've seen yet (but I did skim it so I probably missed a clear-thinking soul here and there). Well, ok, it is probably a WTF to accept null as a value in this circumstance, but given that behaviour (treat null as a value, and changing to/from null should set the dirty flag) the original code beats most of the following attempts.

If you compare Boolean (or any reference type, actually) with != you are comparing the references, not the object contents like the original function does.

This makes the assumption that recordLocked is a Boolean. If it's of the primitive type boolean and Java 1.5 is being used then autoboxing will come into effect and simple object equality == would work fine.

If you compare Boolean (or any reference type, actually) with != you are comparing the references, not the object contents like the original function does.

This makes the assumption that recordLocked is a Boolean. If it's of the primitive type boolean and Java 1.5 is being used then autoboxing will come into effect and simple object equality == would work fine.

Unfortunately, you can't autounbox a null Boolean. The result will be a NullPointerException.

The original code is messy because the "flags" are three state variables that probably should have been primitive types rather than Objects. Most likely the developer is using null as an uninitialized indicator rather than maintaining two separate flags for the value and the "is initialized" state.

This forum really is a haven for complete idiots. Only about three of the commenters actually understood what this code was doing (and I suspect the submitter didn't either). When they are going to so much effort to support tri-value booleans they probably have a reason for it. This code is slightly ugly but is not broken, unlike most of the suggested replacements.

For anyone using Java who needs to compare possibly null objects may I suggest Jakarta commons-lang which include the Object.equals methods in a previous comment as ObjectUtils.equals (along which a bunch of other useful stuff which probably should be in Java - and some complete crap of course).

This forum really is a haven for complete idiots. Only about three of the commenters actually understood what this code was doing (and I suspect the submitter didn't either). When they are going to so much effort to support tri-value booleans they probably have a reason for it. This code is slightly ugly but is not broken, unlike most of the suggested replacements.

For anyone using Java who needs to compare possibly null objects may I suggest Jakarta commons-lang which include the Object.equals methods in a previous comment as ObjectUtils.equals (along which a bunch of other useful stuff which probably should be in Java - and some complete crap of course).

The 1st is the original. The 3rd one shows you can't do a simple "!=".

The 2nd one is a slightly simplified version of the original. The only difference is the "newValue == null" check is redundant, since if newValue IS null it will NOT be equal to isRecordLocked, because we already checked that isRecordLocked is definitely NOT null.

This is the best I can do... if anyone has a simpler function that passes the test (using Boolean types, not boolean primitives), let me know. If not, as far as I'm concerned this code is perfectly fine except that one little redundant "newValue == null" check.

(This is all assuming the original code was in Java and wasn't translated from another language...)

There are a couple of utility classes that would certainly help in this case. Check out HashCodeUtil and EqualsUtil on Google and you get the idea.

The main reason for these "one-liners" to go through specific utility classes is that you don't want any nullpointers to occur in equals or hashCode functions, that is just a very costly mistake to make (either in production or during development).

The other reason is that you can change system policy for Dates for example using these utility functions. For example a function "isGreatherThan" for two dates.

What if the source date is null, does that mean infinity or actually null? How should the second date compare to the first? A utility function makes this policy consistent, rather than an invocation of the same functionality in each place. I'm recently looking at these simple "one-liners" and find that a utility helper function is a great help for establishing these policies. Also, as soon as you want to construct objects and wish to add some functionality, changing the function prototype with modern IDE's will automatically flag any instances where these objects are being used.

For the above, I'd actually consider using proxies, interceptors or AOP and see how this performs against manually invoked code. For example, every object of interest could become wrapped automatically and contain this "dirty flag". Any method invoked that would change a property can then individually set the object to dirty without any concern to the programmer.

Then again... the name "isRecordLocked" in code sort of bothers me. is this Enterprise thing trying to mimic a database or what?

The Boolean object can only have 2 values either true or false. When the object reference is null, it means we dont have a Boolean object yet (perhapes because at the moment we dont know what value it should take, so havent created it yet)

Makes perfect sense to me, I cant see a third value in the Boolean object no matter how hard i look.

On topic, I guess all the attempts show that sometimes what we think is a piece of cake, is sometimes really a can of worms. (Watties, extra cheesy, are my favorite.)

Makes perfect sense to me, I cant see a third value in the Boolean object no matter how hard i look.

A simple example:

You have a database. The DB has tables. A table can have booleans. And they can have three states (NULL, true, false). So your system has to handle the tristate DB-Values.

Or in a general speaking: Whenever you have to create an empty object which will be filled with data by a different system (other computer, other object, other ...) you should set the data to a default NULL value. Otherwise you wont know whether the "fill up" worked or worked not. Thats what we call "defensive programing".