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...

Tuesday, July 8, 2008

Code Naming Rules

I believe it is a fundamental misunderstanding of nature to assume that rules are not made to be broken. This is the sticking point where breakdowns in communications occur - so I'm letting you know what side of the debate I'm on right away. That said - here are some rules I adhere to when naming things - giving real (poor) examples from the system I currently work on:

Example: CallRequestRecipientObj
Reason this is bad: Classnames should never contain the word "Object" or "Class" or their derivatives - unless you are writing framework code and referring specifically to an object or class type. We know it's an object or class we're talking about. Adding "Class" or "Object" into a classname is just redundantly redundant.
Rename to: CallRequestRecipient is sufficient.

Example: GenericPersistableClass
Reason this is bad: For the love of Jesus, don't use the word Generic or Framework in your classnames. Yes yes - your little framework is very clever and flexible and "generic". Stop beating me over the head with it. Especially when naming interfaces... which are, by definition, generic. Also: remove "Class" (see above).
Rename to: Persistable ftw.

Example: interface Account { int getAccountId(); }
Reason this is bad: Don't prepend the object name to an ID. Again, it's just redundant. The argument used to be surrounding database joins. But that's precisely what aliases are for:
select a.id, d.id from Account, Details where a.id=d.accountId
Rename to: getID() is terse and descriptive. It's also the same for every object.

These are a few good naming rules I stand by. Do you have any more? I'd love to hear them. Again, rules are made to be broken - but should be followed, like, 99% of the time - give or take, depending on your rules. Next on the docket: package, project, and namespace naming rules.