Object Commando languages, development and design

12Feb/094

Clean Code By Robert Martin – Part 1

I'm just started reading through Clean Code by Robert C. Martin. It's a book on writing good software at a low level. Things like how to name variables, how methods should look etc. His examples have been in Java, but the concepts are very general. I like what he has to say in the book and I agree with a lot of what he says. I have some comments on things I found interesting below.

Switch Statements

He shares a dislike for switch statements like I do. He gave the rather typical refactoring scenario of changing the switch code to be more object oriented through an Abstract Factory and Polymorphism. The example code he gave was:


public Money calculatePay(Employee e) throws InvalidEmployeeType {
  switch (e.type) {
    case COMMISSIONED:
      return calculateCommissionedPay(e);
    case HOURLY:
      return calculateHourlyPay(e);
    case SALARIED:
      return calculateSalariedPay(e);
    default:
      throw new InvalidEmployeeType(e.type);
  }
}

This is a pretty text book refactor to an Abstract Factory. Create a polymorphic method like "caluclatePay()" and have the employee subclasses provide their implementation of it. In the book he discusses burying that code deeper in the stack. In the Enterprise Java world, I find myself puting that logic into Hibernate. In this case, the Employee has a type, COMMISSIONED, HOURLY and SALARIED. Typically what I do is have the Employee become abstract and in the Hibernate mapping, indicate the descriminator as maybe "employeeType". Then I map all three employee types as their own subclass of employee (maybe using single table inheritence). I end up with three more classes CommisionedEmployee, HourlyEmployee and SalariedEmployee, but they should probably be pretty small classes. With this set up Hibernate does the dirty work previously done by the Abstract Factory described in Clean Code.

How to Name Your Interfaces

This always seems to be a hot button issues. I'm not entirely sure why. There is the more straightforward case of several up front implementations of an interface. For example in a bank scenario, you have Account and implementers of Account are maybe SavingsAccount, CheckingAccount etc. What seems to cause controversy is when initially there is only one implementation. As an example, maybe there is an Account Service. In Clean Code, the suggestion is having AccountService and then AccountServiceImpl. As a nice counter example, in Implementation Patterns by Kent Beck, I remember him taking the approach of IAccountService and AccountService. In general, I have found fierce opposition to prefixing interfaces with I. My personal preference is, if it can be named differently (such as Account above) it should be, if there's only one, I prefer Kent Beck's approach. To me, it seems like Impl is redundant and doesn't tell me much. Every implementation of the interface is an "impl". However, I have found that there are far more pressing issues in the code than whether or not to have Impl on the end of a class name.

Comments

After reading through the chapter on comments and giving it some thought, I agree with a lot of what he has to say. One quote in particular at first struck me as odd, "The proper use of comments is to compensate for our failure to express ourself in code. Note that I used the word failure. I meant it. Comments are always failures. " But when I thought about it, it made a lot of sense. The code is what matters, it's what is executed and in the end, comments are just decoration. We know the code has to be up to date, the comments are questionable. If I'm putting comments in to help understand the code, then the code is not very understandable on it's own, and I should work to make it better. Several times he mentions that when too many comments appear, or the same comment appears too frequently, we automatically block it out. I agree with this and I find myself blocking out most comments most of the time. I think the reason why these noisy or redundant comments make their way into the code because of a culture of comments. Early on, we are taught that comments are good and we should write them. So when a code review happens, and there's no comments on the methods of a class, there's usually a "oh, there should be a comment here" response. It usually has nothing to do with the actual method and most of the time (myself included), we don't ask, why there should be a comment there. Is it because the method name isn't intention revealing? Is the parameter or return type ambiguous?

Comments (4) Trackbacks (0)
  1. I’m curious if you would get the same strong objection to IAccountService if you renamed it AccountServiceInterface. Conceptually, it is exactly the same thing. I’m curious if people just object to the I in front of the name, and not the concept.

  2. If the comment is telling the reader WHAT the code is doing, then I generally agree with Uncle Bob. To me the more compelling case for a comment is to tell the reader WHY the code is doing what it’s doing. Most of the time the motivation should be self evident, thus rendering a comment unnecessary. However if you look at your just-completed code and realize the next poor sap will not be able to figure out why you did what you did, that means it’s a candidate for:
    1. an comment explaining the WHY, not the WHAT
    2. a refactor
    3. both

  3. I’m not sure. Certainly more verbose. Maybe the reason I like the I in front is that there is minimal to ignore when I don’t care about that detail. For the suffix of Interface, seems too verbose, but it would be interesting to see if it gets a knee jerk reaction like the I does.

  4. Is I for “Interface” or “Impl” – haha


Leave a comment

(required)

No trackbacks yet.