In the last article I introduced the highly recommendable book ”Clean Code“ written by Robert C. Martin. While I tried to essentially describe what the book is about and what’s so great about it, I would like to comment on some of his so-called ”code smells“. I do not want to challenge the quality of the book, but I am just act upon one of his key rules: ”Try and leave the code a little better than you found it!“
This rule is heavily biased by the personal style of the author and if you read the longish explanation for it, it doesn’t bring forward any well-founded arguments. Specific imports would supposedly introduce hard dependencies, because the particular class has to exist, while this wouldn’t be the case with wildcard imports. Additionally, the lists of imports would be very long otherwise and would disturb the reading flow. The former isn’t really an argument, because after all, the import doesn’t create a dependency, but the use of a imported class after the import. That means, that even if you supposedly avoid the dependency on JFrame by using e.g. import javax.swing.*, you still have that dependency, because there is something like ”public class MyJFrame extends JFrame …“ only some lines further down. In the end, every class you use in your code in some way has to exist, otherwise the code just does not compile. And due to modern IDEs, unused imports are quickly removed, should the code not compile at some point just because of some non-existent class. And those IDEs also help to weaken the second argument, just by collapsing long import lists.
This is definitely a good rule and should be followed. I only think that the example given is somewhat unfortunately selected:
public ObjectOutputStream getOos() throws IOException { if (m_oos == null) { m_oos = new ObjectOutputStream(m_socket.getOutputStream()); } return m_oos; }(Clean Code, p. 313)
Here, Martin criticizes that the method does more than just retiring an OOS; It also creates one, if it does not already exist and therefore it should better be named ”createOrReturnOos“. Besides that the new name isn’t really that good either, it would reveal very much about the implementation of the method. In the end, what happens here is just some simple form of lazy initialization, which indeed should be used with caution, as he quite rightly writes somewhere else in the book, but is an implementation detail at this point. It should not matter to the caller if the object was just created or at some earlier time. The method does what it promises to do, that is to return an OOS. How it does that, should be none of the callers business. Would the initialization strategy change later, every caller of the method would have to be changed to, because in that case the name would have to be changed too.
This is actually a very simple rule, that every computer science freshmen should learn and therefore also isn’t missing in this book. But Martin justifies two exceptions with very shaky arguments:
For example, the number 86,400 should be hidden behind the constant
SECONDS_PER_DAY. […]double milesWalked = feetWalked / 5280.0; int dailyPay = hourlyRate * 8; double circumference = radius * Math.PI * 2;Do we really need the constants
FEET_PER_MILE,WORK_HOURS_PER_DAY, andTWOin the above examples? Clearly, the last case is absurd. You might quibble about theWORK_HOURS_PER_DAYcase because the laws or conventions might change. On the other hand, that formula reads so nicely with the 8 in it that I would by reluctant to add 17 extra characters to the readers’ burden. And in theFEET_PER_MILEcase, the number 5280 is so very well known and so unique a constant that readers would recognize it even if it stood alone on a page with no context surrounding it.(Clean Code, p. 300)
The reasoning against WORK_HOURS_PER_DAY seems a bit arbitrary and also contains arguments to why it should better made a named constant! Anyway, to leave the number 8 as is in that case, because it supposedly looks so nice, isn’t a good decision. If you start allowing exceptions of this very subjective nature, you are at risk of justifying everything somehow.
The reasoning against FEET_PER_MILE is understandable for an US-american, but shows a lack of international understanding. While someone who grew up with the imperial unit system knows the meaning of the number 5280, the rest of the world does not. Because you never know who might work on your source code, you should avoid to introduce regional dependencies like this and, if in doubt, opt for a named constant instead. In addition, the number 5280 might be also used with a different meaning (e.g. a port number). It also feels a bit inconsistent, when Martin naturally considers SECONDS_PER_DAY necessary, even though this constant could be understood world-wide in the context of time (but, of course, I agree with him that this should be a named constant).
To conclude, these comments show that something like good and clean code cannot be written simply by strictly following some rules, but instead depends on the ”code sense“. The ”sense“ for those things that improve or hurt the read- and maintainability of code. Additionally, you can learn that you should constantly think about the reasons why you chose some style, no matter how good your own ”code sense“ supposedly is.
This entry was posted on September 21st, 2010 at 2:22 PM. It is filed under General, Programming and is tagged with agile methods, book, code, software development. You can follow any responses to this entry through the RSS feed. You can leave a response, or trackback from your own site.
