Thursday, January 7, 2010

Code Smells

My favorite tumblelog trivium (which is where I saw the links to the commentary on Shu Ha Ri that inspired this blog) just posted a link to Private Methods are a Code Smell, where Kent Spillner proclaims:
Private methods are a code smell. They should be moved to collaborating classes and made public.
His reasoning is interesting and concise and I highly recommend it. However, I also disagree, and think it is worth a post to explain why.
First, let's look at one independent definition of what  a code smell is. The Wikipedia entry says:
In computer programming, code smell is any symptom in the source code of a program that possibly indicates a deeper problem.
Given this exact definition, I have to say that Kent is correct. However, it is because the word possibly in the definition leaves a rather large loophole. Yes, there are times when private methods should be refactored as Kent states, extracting them as public methods of collaborating classes. But I read Kent's post as taking a stronger stance. It is as if he almost titled his post Private Methods Considered Harmful. Of course, I may be mistaking his intentions, but let me pursue this anyway.

Recall the three phases of acquiring mastery: Shu Ha Ri. In the Shu phase, a beginning programmer must try to strictly adhere to solid foundational principles or rules. In the Ha phase, the programmer explores the boundaries of the rules, and learns when it is justifiable to break the rules. In the Ri phase, the programmer becomes free of rules, and simply does what is correct in the given situation.

There are many possible principles of programming. Beginning programmers should start with a small number of simple principles, such as: member data should always be private. This is an excellent principle for a Shu programmer to follow. A Ha programmer might experiment with occasions to violate the principle, but will probably learn through experience that every violation of the principle is a mistake.

Let's now consider another candidate principle: member functions should always be public. This candidate principle is close in spirit to Kent's proposed code smell, but Kent didn't state the code smell this way, and for good reason. This would be a bad principle to impose on a beginning programmer. Beginning programmers should follow principles that lead them to avoid repetition, develop an appreciation for encapsulation, and learn how to create narrow interfaces. But more importantly, beginning programmers should learn the principle to keep it simple.

But Kent didn't propose a fundamental principle. He proposed a code smell. So is it a good code smell? Consider the first five examples on the Wikipedia page I linked to earlier:
  • Duplicate code: identical or very similar code exists in more than one location.
  • Large method: a method, function, or procedure that has grown too large.
  • Large class: a class that has grown too large, see God object.
  • Feature envy: a class that uses methods of another class excessively.
  • Inappropriate intimacy: a class that has dependencies on implementation details of another class.
These are all excellent code smells that are appropriate to teach to Shu level programmers. The private method code smell is another beast entirely. I can see how it might be a good code smell to give to Ha programmer, posing it as a learning exercise: Consider private methods as a code smell, and the recommendation to refactor them into public methods of collaborating classes. Is this a good universal rule? If not, what are the exceptions to the rule?

A Ha programmer who spent a few weeks with this exercise will very likely come out of the exercise as a better programmer. But if they conclude that the rule is a good universal one with very few exceptions, then I suspect they aren't yet nearing the Ri phase of programming. Your humble 4-dan programmer thinks private methods should only be considered a smell in the presence of other smells, i.e. the decision to refactor one class with private methods into two or more collaborating classes with public methods shouldn't be prompted by the presence of private methods alone.

Another of my posts might apply here: The Siren Call of Abstraction. In that post, I warn that programmers often fall into a kind of trap, where they are seeking a kind of purity through abstraction and generalization. The concept of code smells is a useful one, but I sometimes wonder if the art of programming doesn't also need a concept we might call purity smells.

One favorite principle of mine is: avoid speculative complexity. This principle deserves its own essay, but let me just say here that many programmers fall into a trap of speculating about future requirements of their applications, and attempt to design their applications so that those future requirements will be easy to implement when the time comes to do so. Programmers who have become enamored with refactoring might be inclined to do premature refactoring, which might be nearly as big as a mistake as doing premature performance optimization. I love refactoring as a tool, but for some time now, my refactoring has always been driven by the pragmatic concerns of doing what it takes to achieve my project's near term goals. With this style, there are a number of code smells that I find natural and intuitive, but calling private methods a smell seems a bit forced.

No comments:

Post a Comment