Friday, May 21, 2010

Self Documenting Code (Because I Hate Maintaining Comments)

Suppose you coded up some phone switch record parsing software a few months ago and now you're modifying it and you run across this method. What kind of records does it skip?


/**
* @return true if we should skip (NOT add to the database) the record.
*/
public boolean skipEntity() {

if ("119".equals(Call_Type_Code)
&& !Incoming_Trunk_Id.startsWith("10101")) {
return true;
}

if ("005".equals(Call_Type_Code)
&& "00001".equals(Structure_Code)) {
return true;
}

return false;
}


You probably wouldn't know without looking it up in your data dictionary. So let's add some comments:


/**
* @return true if we should skip (NOT add to the database) the record.
*/
public boolean skipEntity() {

// Skip terminating access

if ("119".equals(Call_Type_Code)
&& !Incoming_Trunk_Id.startsWith("10101")) {
return true;
}

// Skip local calls

if ("005".equals(Call_Type_Code)
&& "00001".equals(Structure_Code)) {
return true;
}
return false;
}


Oh ok, we skip terminating access and local call records. That was easy.

Problem: Now we have comments to maintain in addition to the code. Comments can be as hard to maintain as code. You have to make sure they stay with the correct block of code, that they still correctly describe the code, etc.

So instead, let's make the code self-documenting by splitting it up into more methods.


/**
* @return true if we should skip (NOT add to the database) the record.
*/
public boolean skipEntity() {

if (isTerminatingAccess()) {
return true;
}

if (isLocalCall()) {
return true;
}

return false;
}

public boolean isTerminatingAccess() {
return ("119".equals(Call_Type_Code))
&& !Incoming_Trunk_Id.startsWith("10101"));
}

public boolean isLocalCall() {
return ("005".equals(Call_Type_Code)
&& "00001".equals(Structure_Code));
}


Is it a little more code? Yes. Is it more readable and maintainable code? Yes.