Tuesday, October 06, 2009

Another note on code readability

Some time back I posted a piece of code I found that was trying hard to obscure its meaning (see this post from September 2008). Today I came across this:

long now = CalendarUtil.theCalendarUtil().getCalendar(new Date()).getTimeInMillis(); 
long milis = now - creationDate; 
milis = (milis < 0 ? milis * (-1) : milis); 
return (milis > 2 * 60 * 1000);

Now, do you immediately see what’s going on here?

Right – of course, you do. The method is called isInstanceTooOld(), the idea being that it should return true, if the current object was created less than two minutes ago. The constructor records the creationDate field to track that.

This nicely shows that convoluted code – even intended to provide a simple function like this one – is prone to errors: This one returns true, too, if the current object is two minutes newer than the current time. The correct function looks like this:

static final long MAX_AGE_THRESHOLD_MILLIS = 2L * 60 * 1000;
long now = System.currentTimeMillis(); 
long age = now - creationDate; 
return age > MAX_AGE_THRESHOLD_MILLIS;

Always a pleasure to see the face of people when they recognize what strange things they did only weeks ago, probably late at night :)

Technorati Tags: ,,,

1 comment:

Federico said...

nice ;-)

guess everybody's code is full of that! sometimes it is usefull to review your own code after some time to fix things like that!

thanks for reminding me to do it ASAP :-)