Let’s play a game.
The rules are simple: write some code. Oh, and you have to do it right.
That means if you break one of the following rules, courtesy of the
brilliant Adrian Bolboaca, someone will
be there to slap your wrist.
- Lack of tests
- Name not from domain
- Name not expressing intent
- Duplication of constant
- Method does more than one thing
- Primitive obsession
- Feature envy
- Method too long (> 6 lines)
- Too many parameters (> 3)
- Tests: not unitary
- Tests: setup too complex
- Tests: unclear action
- Tests: more than one assert
- Tests: no assert
- Tests: too many paths
That’s a lot of rules. The idea is to have a bunch of people hovering
around, looking out for these code smells. A smell being a hint of a
problem, not a problem in itself, which means that even something that
looks fine can still be flagged. Once something is highlighted, you have
to stop and fix it immediately. You can’t wait.
There’s a reason Adi calls this game “brutal”.
I have to apologise for taking so long to write this post. We ran this
session at the
LSCC almost a
month ago, and I think it went really well. People worked in pairs on
implementing tic-tac-toe in whichever way they deemed the “right” way.
We weren’t so fussed about the approach—while design is important, it
wasn’t the focus of this exercise. What was interesting was the routes
people took to implement their solutions: they all ended up breaking one
of the rules, despite the fact that they were up on the big screen right
in front of them. And you have to wonder, why is that?
The rules people broke the most were #2 (name not from domain) and #4
if). I have to admit, I’m a bit harsh when it comes to
conditionals: I’d argue that no
if is necessary when implementing the
rules for a game such as tic-tac-toe. They are when you’re dealing with
user input, but that’s about it, and no one got that far in an hour and
a half. I’d be incredibly impressed if they had. People hated it when I
flagged this, but they mostly found that the end result implementing it
without (using polymorphism, for example) was much cleaner. It usually
ended up moving behaviour from many places into one and giving it a
name, Which conveniently solved #2: that new fellow almost certainly
got a domain-specific name.
The majority of these rules stem from two areas: duplication and a lack
of domain clarity. Some are obvious, such as #6 (duplication of
constant), but some need a bit more thinking about. If you have a method
that does more than one thing (#7), it’s almost certainly going to be
named according to its behaviour and not its responsibility in the
domain, which means that the language used to describe it is unlikely to
be shared among the entire team. Focusing on extracting domain objects
that do one thing and do it well solves this one and most of the others.
If it’s well-tested, that really does take care of the rest.
There are a few other personal favourites among this set of rules. #8,
primitive obsession, is something that’s extraordinarily hard to fix in
the LSCC’s favourite languages, Java and C#. Wrapping concepts such as
identifiers and HTML in objects just makes them harder to deal with, and
Price objects for your prices rather than passing around
doubles makes the arithmetic so, so ugly in Java. (Gotta love operator
overloading.) Another one is #13 (tests: setup too complex). I’m
surrounded by tests where the entire world is mocked (a side effect of
#11—too many parameters), and I spend far too much time rewriting them
to construct what they need and mock only what’s necessary. This makes
it so much easier to break the object apart later.
This game is a rebellion against the attitude of “I’ll do it later”—the
mindset that causes us to leave code in a terrible state because we just
can’t be bothered to fix it right now. Just because step three of the
TDD cycle is “refactor” doesn’t mean the red and green steps can be
shoddy, because we don’t always refactor. As Jeffrey Mayer says, if you
haven’t got the time to do it right, when will you find the time to do