Summary
Few things bug me more than code that's not obvious. And what it takes to make things obvious seems so, well, obvious.

I was subclassing something the other day and while looking at the super class to figure out how to do that I ran into a chunk of code that look like this:

syncronized(aList){
  object = aList.get(0);
}


synchronized(aList){
  aList.removeFirst();
}


I'm looking at this thing and asking myself why 2 synch blocks? It's so obviously there it has to have a reason. I didn't see it though. I read the java docs for List. Maybe I didn't know something fundamental about how those worked. Nope. I look at the users of the class. No they didn't need the behavior...

At this point I'm obsessed. This double synch thing is offending me beyond all reason and it better justify it's existence damn quick 'cause I'm gonna commence to chopping it out. Time to break out the big guns and go ask someone else for an opinion. It was lunch time but luck was with me (or against that little code snip) and someone worth asking was around. I point. He nods. He thinks. He looks up. He looks down. Nope, he can't see any justification for this little guys existence either. Excellent.

There's no doubt this annoying snippet is trembling now I thought. Time to get de-rezed you itchy, what the hell are you doing in the middle of my day, useless chunk of code. Mark, cut, compile, sub-class, test, check in. Woohoo! Getting rid of useless code is one of my favorite things to do. The rest of the day is just that much more satisfying.

The next day this mail pops into my box. Uh-oh. It's a forward of my checkin mail (our source control system mails on each checkin). This is almost never good news. Sure enough the double synch thing had a purpose and broke something. Turns out there was a method, maybe 40 lines above the double synch that relied on the object being in the list until after the doSomeStuff() call.

So I back out my change (Tenacious snippet. I heard it laughing as it slimed it's way back out of the bit bucket.) and add a comment to the code. The comment was this:

// This double synch thing is used by the method XYZ() above

This reminds me of door handles that look like they should be pulled, and you do, and it's only then that you discover that you have to push. Look, if your design esthetic is such that your creation could be non-obvious to others I'm ok with that. But could you put a Push sign on the door? How hard is that?

Talk Back!

Have an opinion? Readers have already posted 15 comments about this weblog entry. Why not add yours?

RSS Feed

If you'd like to be notified whenever Rick Kitts adds a new entry to his weblog, subscribe to his RSS feed.

Digg | del.icio.us | Reddit

About the Blogger