Test-driving methods in C# or Java often requires variant values for a given primitive argument:
[Test]
public void ShouldGenerateAppointmentsOnMonthBoundariesByDefault() {
var appointments = new QuarterlyRule(4);
Assert.That(...);
}
[Test]
public void ShouldGenerateAppointmentsOnSpecifiedDayOfMonth() {
var appointments = new QuarterlyRule(2, 15);
Assert.That(...);
}
The numbers (4, 6, 15, …) are magic, but there are numerous ways of clarifying things for the reader. We could “encode” them in the test name, but I’ve found that simply clutters the name, making it even harder to quickly digest. Test names should be as implementation-free as they can be, within reason. They should capture the behavior in an abstract sense.
We could introduce a variable–either local or more global–that clarifies things. Here are two examples:
[Test]
public void ShouldGenerateAppointmentsOnMonthBoundariesByDefault() {
const int recurrences = 4;
var appointments = new QuarterlyRule(recurrences);
Assert.That(...);
}
const int RECURRENCES = 2;
[Test]
public void ShouldGenerateAppointmentsOnSpecifiedDayOfMonth() {
var appointments = new QuarterlyRule(RECURRENCES, 15);
Assert.That(...);
}
Hmm. The problem with the global constant in this case is that it imparts only half the meaning. If the 6 is relevant to the ultimate assertion (and it probably should be in order to be part of a meaningful test), and that value is hidden, it forces the reader to navigate in order to completely understand the test. We can rename the constant:
const int TWO_RECURRENCES = 2; // or ...
const int RECURS_QUARTERLY_ACROSS_HALF_YEAR = 2;
… but those names are unsatisfying. The first is somewhat silly, and the second is too clever, demanding a bit too much effort from the reader.
An explaining temporary variable can be acceptable, but it is disappointing to waste an entire line just to document an argument, particularly if you have a good number of tests that require the variance. And particularly if you have tests that would otherwise be one-liners or three-liners (arrange-act-assert).
Other solutions? You could stuff a comment (horrors!) into the call:
var appointments = new QuarterlyRule(/*recurrences*/ 4);
No thanks. You could encode the parameter in the method name (in this case, using a constructor factory method):
var appointments = CreateQuarterlyRuleWithRecurrences(4);
Sometimes that’s appropriate, other times it is burdensome to the production code. You could punt and use a better language:
appointments := QuarterlyRule new recurrences: 4 dayOfMonth: 15.
Am I missing any other options? Perhaps. The one I chose to do today, in an act of whimsy:
[Test]
public void ShouldGenerateAppointmentsOnMonthBoundariesByDefault()
{
var appointments = new QuarterlyRule(Recurrences(4));
Assert.That(...)
}
int Recurrences(int number) { return number; }
Is this nuts or what? So far I’ve used it in one test class; I can imagine it could become onerous to create numerous such methods. But for now I plan on constraining use of it to situations that require pervasive use, across a good number of test cases.
Thoughts? Rotten eggs?
Comments
Jeff, it seems to me that this small difficulty is alerting you to a slight smell in the QuarterlyRule constructor. Doesn’t non-test code have this same difficulty of making the meaning clear?
Hi George,
I thought about it as a constructor design smell last night after I went to sleep, but wasn’t able to think of a great way to eliminate it.
My “schoolin’” told me to view constructors as ways of creating objects that were in a sufficiently valid state–in this case, for example, a QuarterlyRule is of limited use if no one specifies how many times it should apply. But lately (like for many years now), given the power of having unit tests that document things, I don’t think there’s any good reason to enforce that rule–tests can document the valid protocols for multi-step construction of objects. So maybe the better way to go is with no-arg constructors and properties? I dunno. It seems to be these “little things” that I waiver on and that ultimately are pretty pertinent.
Non-test code would rarely have the same difficulty–you’re not typically calling a method with some arbitrary constant, and even if so, it wouldn’t be so pervasive. So an extra line for a temporary wouldn’t be clutter.
Jeff
I felt the same goofyness the last time I did the bowling Kata: to make tests like the following more readable:
@Test
public void perfectGame() throws Exception
{
roll(10,12);
assertThat(game.score(), is(equalTo(300)));
}
I introduced the following methods:
private int pins(int n){ return n; }
private int times(int n){ return n; }
just so I could replace
roll(10,12)
with
roll(pins(10),times(12))
I’m late to the party, but funnily enough a couple of months after this posting a fellow in Germany posted about the same thing (improving readability through methods that simply return their argument); he called them “code squiggles”:
http://schneide.wordpress.com/2010/06/28/add-flair-to-your-code-code-squiggles/
Since both of these conversations happened about a year ago I’m curious: How do those who have tried it feel about this technique a year on?
Thanks for the link Peter. As mentioned here, my goofy construct was indeed an act of whimsy. In similar situations later, I’ve resorted to other techniques (more often than not fixing the method name).
Two possibly better options: QuarterlyRule.recurrences(4) or make Recurrences a class and see whether it attracts any code.
Thanks JB–great ideas. I definitely like “Replace Constructors with Creation Methods” (Kerievsky).
Making a Recurrences class is also a fun idea and isn’t just limited to constructors. I’ve been pleasantly surprised far more often than not when I introduce a new cheesy little abstraction like that.
Thanks to Brian Ward for pointing out this idiom for C++:
#define Recurrences
…
new QuarterlyRule(4 Recurrences);
#undef Recurrences
Seemed effective for a handful of tests we wrote today.
