Gian Lorenzetto presented an insightful and thorough review of “Implementation Patterns” by Kent Beck last night at the Melbourne Patterns Group and it reminded me of discussions I have had with the development team on making code more understandable and therefore more maintainable. What follows is the gist of the discussion ….
The systems we build are ore more organic than they are mechanical. Brent said this better than I and described software systems as having an entropy.
This is a very good analogy, since we grow applications and maintain them over time. When left neglected they take more time to maintain and bring back to life or move in another direction. Like a garden.
I think it is worth pondering the above for a little while.
Related to the above is this code I saw in an application:
void setMessage(String message) {
if (message.length() > MAX_SIZE) {
this.message = message.substring(0, MAX_SIZE);
} else {
this.message = message;
}
}
I have the following observations about this code and suggested improvements to make it more clear and therefore more maintainable:
(I know it is a simple setter but this shouldn’t remove the need to make more clear)
1. It is surprising that a setter also truncates
2. MAX_SIZE of what ?
Here it is applied to the message length, but can it be applied to another place, maybe.
A name that would be more clear would be MAX_MESSAGE_SIZE. When someone sees it declared as a constant at the top of the file they will immediately know what size we are talking about and that they should not use it when dealing with the maximum size of a subject line.
2. The complexity of the setter is higher than it needs to be and in solving this complexity we can also make the method more clear. Currently there are two paths through the method, meaning we need more tests around it and it is harder to understand.
Here is a suggested alternative that I think is more clear and reduces complexity:
void setMessage(String message) {
this.message = truncateTo(message, MAX_MESSAGE_SIZE);
}
String truncateTo(String thingToTruncate, int sizeToTruncateTo) {
if (sizeToTruncateTo < thingToTruncate.length()) {
return thingToTruncate.substring (0, sizeToTruncateTo);
}
return thingToTruncate;
}
The method truncateTo() probably belongs elsewhere in a String utility class, making the code in this class simpler again. I haven’t just moved the conditional elsewhere, since it doesn’t have multiple paths through the code, so the npath complexity is lower.
Even simple setters can be made more simple and more clear, which helps us understand and grow our garden.
