Friday, December 09, 2011

31 Days of Testing—Day 9: Readable Tests

Updated: Index to all posts in this series is here!

This post is an off-the-cuff tangential post based on some interesting comments to my previous posts on test fundamentals.

A couple folks commented on my use of C#’s “var” keyword, pointing out that it made the tests less readable in their view. I think it raises an interesting point of discussion that is an extremely large debate on the value of strict typing. I’m going to forgo that conversation, and instead try to focus down on readability of tests.

In the following test, what’s most important?

[Test]
public void Computing_with_40_hours_at_rate_5_returns_200()
{
    WageComputer computer;
    bool isHourlyWorker = true;
 
    var computedWages = computer.ComputeWages(40, 5, isHourlyWorker);
 
    Assert.AreEqual(200, computedWages);
}

This test is validating that the ComputeWages method is correctly working and returning the correct values for my inputs. This test isn’t looking to validate the return type. Frankly, I don’t care what the return type is. I’m more concerned that the algorithm is functioning properly.

If I’m only concerned about the value coming out of the method, then, quite frankly, anything extra at all on the left-hand side of that statement is pure noise to me. Look at these two different examples:

double computedWages = computer.ComputeWages(40, 5, isHourlyWorker);

var computedWages = computer.ComputeWages(40, 5, isHourlyWorker);

That’s how my mind looks at these sorts of things. Frankly, I’d be happier if the compiler just got rid of the entire type declaration. The compiler knows what the right-hand side is returning. Even “var” is just noise to me. Something like this would be a thing of beauty:

computedWages = computer.ComputeWages(40, 5, isHourlyWorker);

(By the way, that’s not my idea. My pal Leon Gersing put this idea in front of me during an interesting discussion several years back at a .NET training day we put on.)

Moreover, explicitly using strong typing makes your tests more brittle. If you ever change the return type from ComputeWages you’ll need to go back and update every test using that class. (In this case that’s a somewhat contrived example. Please look at the larger point here.)

If you feel the need to have a test specifically checking the return type, then write a separate test for that.

Carry this over to everything else about your tests. What’s noise? What’s really important? Can you easily grok a test you wrote three weeks ago? Three months ago?

No? Why not?

Great software engineers, or the latest trend of labeling folks who care as “craftsmen,” understand the criticality of writing code that’s easily readable.  Tests need to have the same care, because tests are production code.

Take a close look at what you really for your test and cut out everything else. It’s just noise.

Updated: By the way, in case you get put off my my frequently curmudgeonly sounding tone, I really do love this sort of feedback and questioning on my posts. I do not have all the answers. Your questions and points make me constantly re-evaluate how I’m doing things, and that’s a good thing! Please, keep up comments and questions. I love ‘em!

4 comments:

Yossu said...

I find it interesting that you end your defence of using var with the words "tests are production code"

Does this mean that you would use var in the app code? If so, then your comments really aren't about testing at all, they are part of the bigger picture of the use of var.

Personally, I never use var unless I really have to, as I like to see a precise piece of code that doesn't require any guesswork as to what it does. Having said that, I wouldn't argue with anyone who likes var, as long as they don't want me to maintain their code!

Thanks for some great articles. Keep them coming.

Jim Holmes said...

@Yossu: Yes, my comments are indeed a larger picture about var. I said as much in my opening of the post.

If a reader can't understand a program's flow because the type isn't specified then there are vastly larger issues causing the problem.

Those same folks are going to have trouble when, not if, I start using Ruby examples in this series...

Glad you're enjoying the series, even if I do use "var." :)

Jasmine said...

First let me say I agree with what you said - it's a trade-off and in this case, using var might be better. It was added to the language for purposes like this - where the type of the function might change.

However, I take issue with this: If a reader can't understand a program's flow because the type isn't specified then there are vastly larger issues causing the problem.

TYPE is super important in object oriented code. If I say...

MyObject o = new MyObject();
o.StartTask();

You are going to need to know the type of "o" to figure out what that code does. This becomes tricky when you do something like this...

var o = GetMyObjectFromFactory();
o.DoStuff();

That kind of thing can be very mysterious, but if you do this...

IDoStuff o = GetMyObjectFromFactory();
o.DoStuff();

Then it's a whole lot more readable. I would also accept this (although it's not as concise as the previous example):

var o = (IDoStuff)GetMyObjectFromFactory();

So you won't convince me that dropping type declarations everywhere is even remotely a good idea. I'm in VB hell a good many days, and I can show you the painful result of not paying attention to types. However, your point about the testing code makes perfect sense, and is one of the reasons var was added to the language in spite of much protest.

Jim Holmes said...

@jasmine: Yes, I've come to understand the importance of types in object oriented programming during my couple decades of working with and around it... :)

That said, your examples serve to underpin my points, quite frankly. There is nothing that's readable or understandable about

IDoStuff o = GetMyObjectFromFactory();
o.DoStuff();

I realized you used throwaways names, but it's still my strong opinion that simply having the type one line above the first usage lends nothing to readability. That's even more dramatically the case if you're using longer classes or methods where the declaration may be a number of lines away or even off the current screen of text. In that case the type in the declaration does you no good whatsoever and you're forced to fly the cursor over the instance to get some type hints (differs in what IDE you're using, of course).

Instead, focus less on the type declaration and more on the naming.

var employeeToUpdate = EmployeeFactory.GetUpdatableEmployee();
var updaterService = ServiceLocator.CreateNewUpdateService();

// several lines of other goo
// like other service calls, security checks, whatever

var updatedEmployee = updaterService.UpdateEmployee(employeeToUpdate);

OK, so my example's also contrived and is mixing concerns in the same block of text, but I'd say it still makes my point. (And frankly I don't care if you're using the strict type names there. The point is the *variable* and method names are oh so much more important!)

Regarding VB hell: been there, done that, though not for many years, thankfully. You're running in to type issues? I'd say have a closer look at your test coverage and test case design. Your tests should be protecting you against exactly these sorts of problems.

I hope you're not taking my strong opinions here as some attack on you. :)

Subscribe (RSS)

The Leadership Journey