Monday, July 21, 2008

Cheap Tricks XVI - Terse Equals

I've been running across a lot of code like this lately. It's something I've brought up with my own developers, but suppose it's just something generally useful. When testing for equality of object, the precursing null check. Here's the code I'm seeing everywhere:
  @Override
public boolean equals( Object obj )
{
if ( !( obj instanceof Account ) )
{
return false;
}

Account that = (Account) obj;
if ( that.id == null || this.id == null )
{
return false;
}
if ( !that.id.equals( this.id ) )
{
return false;
}
return true;
}
I'm a fan of short concise code - and even if you're not... come on... it's an equals method! Let's be frank:
  @Override
public boolean equals( Object obj )
{
if ( !( obj instanceof Account ) )
{
return false;
}

Account that = (Account) obj;
return id == null ? that.id == null : id.equals( that.id );
}
If this id is null, then from the tertiary operator, you only need to know if that.id is null. If so, then they are equal. If not, then you just need to execute the equals method on them. Now if only Java would retrofit equals to use generics...

1 comment:

rattigan said...

Not quite equivalent; null id handling differs. Also, what happens when this == obj. How about this:

@Override
public boolean equals(Object obj) {
__if (this == obj)
____return true;
__if (id == null || !(obj instanceof Account))
____return false;
__Account that = (Account) obj;
__return that.id != null && id.equals(that.id);
}

I think it's important to balance terseness with readability. The original is too verbose, but your replacement is perhaps too terse for its own good.