Not everything deserves a setter
Consider my Bacon class with an IsCooked property:
public class Bacon
{
public bool IsCooked { get; set; }
}To cook some bacon, I set IsCooked to true. Sure. But what if I set it to false? I just defied physics and the space-time continuum. Shucks.
IsCooked should be read-only, and I need a different way to change it in a one-way transition: I need a method. Bacon comes into my domain uncooked, so I'll set that property in its constructor.
public class Bacon
{
public bool IsCooked { get; private set; }
public Bacon()
{
IsCooked = false;
}
public void Cook()
{
IsCooked = true;
}
}The sanctity of thermodynamics is preserved. When the transitions get more interesting than a simple Boolean, you're looking at a state machine.
Polywhatism?
I encountered this example in the wild, but I've changed the domain to protect the innocent.
We're exploring a dungeon, and in our inventory we have items that affect our health:
- amulets add a small amount to our health each day;
- granola bars give us a lump-sum boost to our health the moment we eat them; and
- cursed rings sap away our health each day until the curse runs out. (Why keep a cursed ring? How else would you command your army of winged monkeys?)
Here's a display of our inventory:
| Item | Daily Boost | Total Remaining |
|---|---|---|
| Obsidian amulet | 150 | |
| Luck ring | (25) | 375 |
| Jasper amulet | 253 | |
| Granola of righteousness | 400 |
This was implemented as a collection of IHealthItem objects. Here's what I found:
public class Amulet : IHealthItem
{
public int? DailyBoost { get; set; }
public int? Total
{
get
{
return null;
}
set
{
throw new NotSupportedException(
"Amulets do not have a total");
}
}
}As if the cursed rings weren't bad enough, now there are properties we're not allowed to call, lurking, waiting to pounce on us with a run-time exception. The calling code, when working with an IHealthItem instance, is forced to make an "Is" check to determine the type, in order to know which properties are safe to use. Half the properties aren't relevant half the time. The DailyBoost property on CursedRing has an additional problem: Should you set it to a negative number, or assume it should be stored as an absolute value and multiplied by -1 when being used and displayed? The IHealthItem interface does not provide any abstraction.
In fact, these are not three variations of the same thing. The only quality they have in common is the user's desire to see them displayed in a table. (Further proof that they weren't related, in the real application, that table was labeled "Amulets/Granola Bars/Cursed Rings.") These concepts should have been modeled as three independent entities, keeping it unambiguous in the business layer how to use them to increment and decrement the adventurer's health. Only at the presentation (UI) layer, they could be projected into a display-only DTO (Data Transfer Object) or view model, using strings for all three properties.
Don't let me be uninitialized
If a property must be set for an object to be in a valid state, the class's constructor should require that value as an argument instead. That way, it's impossible to construct the object without also putting it into a valid state.
If a method, MethodB(), uses a private field and relies on another method, MethodA(), to be called first to set it, MethodB should take that value as a parameter instead. That way, MethodB can be called only after the result of MethodA has been determined; the correct order in which to call them is unambiguous.
These types of temporal coupling are illustrated nicely with examples and remedies on Mark Seemann's blog in "Design Smell: Temporal Coupling." (He has a whole series on encapsulation.)
Too, meaning drives syntax
When I find myself compelled to explain in comments, to write instructions ("make sure you set this before you call that") in the xml summary, to incorporate usage notes into method names, I pounce on that as a candidate for refactoring. Finicky methods and classes that require secret knowledge to use correctly are invitations for bugs. It's like when I'm editing the rules for one of Jon's games: every instance of the word "except" must justify its existence. Every special-case exception immediately draws my attention as a spot where the rules could be improved and streamlined. Similarly, when coding, certain behaviors draw my attention as opportunities for improvement. Hyper-explainy documentation is a big one.
Semantics is the study of meaning, and syntax is the study of structure. That distinction makes them sound like separate domains, but the two interpretations of "foreign car mechanic" stem from syntactic ambiguity. Each word still carries the same meaning, but in one case "foreign" modifies "car," and in the other, "foreign" modifies "mechanic." Syntax in natural languages certainly influences meaning, so why not in code, as well?