Everything I know about class and method design I learned from Hannibal Lecter.
OK that's not exactly true, but the first, most basic, and in many ways most important thing that I learned about class and method design I learned from the good doctor. Before there was IoC, dependency injection, MVC, MVP, the
Open Close Principal, or
Liskov's Substitution Principal. Before there was even encapsulation and loose coupling, there was Hannibal Lecter's First Principle:
As software developers we live in a world of unbounded complexity. Most of the design principles that we have developed over the years represent, at their heart, our effort to beat down and shackle that complexity: to build walls around complex systems and declare, "Beyond this point none shall pass!"(encapsulation), to clearly define responsibilities, declaring "A shall do thus, and B shall do thus, and never shall the one take up the task of the other" (abstraction, loose coupling, MVC, MVP). We keep these ideas ever in our minds as we develop the classes and methods that implement the solutions that earn us our daily bread.
But the advice we should turn to most often is that of Hannibal Lecter. His words should ring in our ears throughout each day: each thing that we develop, each method, each class -- what is it? Step back from the problem you are trying to solve for a moment, and look at each one of our little creations in isolation. Each is deserving of its own identity, its own purpose, its own place in the cosmos. Each deserves to have a life of its own, unique and independent of the lives of its brothers and sisters. Software is complex enough without having to remember the particular problem that the author of a class or method was working on at the time he or she wrote it. As Abraham Lincoln once said, "the world will little note nor long remember the context in which the method was written, but the method itself will live on."
Sometimes it's not easy to step back and focus on the individual pieces of our solution in isolation from the rest. It takes a change of gears, a certain adjustment of perspective. In the following clip we see Hannibal guide Clarice in finding the true essence of the class she's working on. As with many of the good doctor's creations, it may be best that we don't try to guess its exact purpose, though it does appear to be a subclass of a class called "Man":
As always, the good doctor gets straight at the heart of the matter. The purpose of this class is not to be found in the particular use to which it's currently being put. It has a more basic, more fundamental nature.
At this point it may be best not to try to draw concrete examples out of the problem Hannibal and Clarice are working on. Instead, I'll give three brief examples from some code I reviewed just last week. The names of the methods and the classes will of course be changed to protect the innocent: these types of problems disturb the good doctor greatly, and it's best not to upset him.
namespace HannibalsFirstPrinciple
{
public class ViolationNumberOne
{
// Called before running the batch process
public void Foo()
{
// Does some stuff
}
}
}
Any time you see a comment like the one on the public method "Foo" shown here, alarm bells should start ringing in your head. You don't have to read the method body to know that Hannibal's First Principle has been violated.
This is a public method on a public class. Who's to say that Foo will always be called before running the batch process? The client can call Foo whenever it likes! And what the heck is "the batch process" anyway? It sounds like some piece of some particular problem the author of this class was working on at the time he or she created it. Clearly the author wrote this with a single-minded focus on one particular problem that he or she was working on, and never took the time to think about
what this class is in and of itself.
This programmer would not be producing code of this sort today if he or she had studied under doctor Lecter. Here's another example:
namespace HannibalsFirstPrinciple
{
public class Account
{
public int Amount { get; set; }
}
public class ViolationNumberTwo
{
private const int MAX_ACCOUNT_AMOUNT = 10000;
public void ValidateAccounts()
{
var account = new Account() { Amount = 5000 };
Console.WriteLine(IsAccountOverTheLimit(account.Amount, MAX_ACCOUNT_AMOUNT));
}
private bool IsAccountOverTheLimit(int value1, int value2)
{
return value1 > value2;
}
}
}
Consider the method IsAccountOverTheLimit. In the context of the entire program, it seems reasonable enough: it checks the amount in an account against the maximum value allowed. But what does it really do? In fact, IsAccountOverTheLimit itself has nothing to do with accounts or limits at all. It simply compares two integers; it's the client that gives these particular meanings to the method. Had the author studied under doctor Lecter (and survived), he or she would have written the method this way:
private bool IsAccountOverTheLimit(Account account)
{
return account.Amount > MAX_ACCOUNT_AMOUNT;
}
Or perhaps this way:
public static bool IsGreaterThan(this int thisNumber, int numberToCompere)
{
return thisNumber > numberToCompere;
}
Either make the method a full-fleshed citizen of the world by giving it all the rights and responsibilities implied by its name, or change its name so that it can declare its true purpose in life with honesty and dignity.
And the final example gets to the heart of understanding the essence of a class, and of maintaining its simplicity:
namespace HannibalsFirstPrinciple
{
public class Account
{
private int _amount;
private readonly bool _isInterestBearing;
public Account() { }
public Account(bool isInterestBearing)
{
_isInterestBearing = isInterestBearing;
}
public int Amount
{
get
{
return _isInterestBearing ? (int)(1.1 * _amount) : _amount;
}
set { _amount = value; }
}
}
}
The programmer here as cluttered up a perfectly good Account class with a lot of extra nonsense that many clients of the class probably don't care about. If you have a special use for a class, specialize it! Don't introduce complexity when simplicity will do:
namespace HannibalsFirstPrinciple
{
public class Account
{
public int Amount { get; set; }
}
public class InterestBearingAccount : Account
{
private int _amount;
public int Amount
{
get
{
return (int)(1.1 * _amount);
}
set { _amount = value; }
}
}
}
These examples seem silly and simplistic because they've been constructed to shine the light of day on these problems, but in the real world these anti-patterns can easily hide amid the complexity of our code, and it's important to note that I saw examples like these in real production code just this week. These types of problems can bite any of us unless we keep our vigilance, unless we keep doctor Lecter's words of wisdom ever ringing in our ears.
Or if doctor Lecter has used too many words for you to remember, then at least remember this exhortation by one of his students, who has clearly grasped the essence and urgency of his lesson:
Safe and happy coding all!
