Monday, June 18, 2007

Four ways of doing a test with a result.

I've been writing a framework for processing emails. These emails can be rejected for various reasons, but when they're rejected we also need to get the reason for the rejection too. It's a common and simple scenario where you need test a boolean value and get some information, but what's the best way of going about it? Well if you're going to be returning more than one thing from a method, in our case the boolean result plus the reason, the easiest way to do it is to wrap them up in a result object. Here's my first attempt:
RejectResult result = email.IsRejected;
if(result.Rejected)
{
    DoSomethingWith(result.Reason);
}
But I don't really like this because the if statement looks like it's saying "if result is rejected". The result's not rejected the email is. I'd like the code to say "if email is rejected", so how about this, using out parameters:
Reason reason;
if(email.IsRejected(out reason))
{
    DoSomethingWith(reason);   
}
OK, this is a bit better, but now it's saying "if email is rejected reason", leaving out 'out' which is just syntax, which still isn't what I really want. Also there's a practical problem here that my favorite unit test mocking framework NMock2 makes a real meal out of out parameters. OK, so how about this one:
if(email.IsRejected)
{
    DoSomethingWith(email.RejectReason);
}
The if statement reads nicely now, but there's a serious practical problem in that we're not expecting email's state to change between testing for rejection and examining the RejectReason, this would be especially serious if email wasn't thread safe but even if it is it's relying on convention; what does RejectReason mean before IsRejected has been tested? Not wrapping the IsRejected and RejectReason in a single method of email is just plain bad. My favorite solution was suggested by NMock2. I really like its conversational style, 'Expect.Once.On(mymock).With(myparam).Will(Return.Value(myreturnval));', so how about this:
Reason reason;
if(email.IsRejected.For(out reason)) 
{
    DoSomethingWith(reason);
}
It reads really nicely "if email is rejected for reason", mocks nicely because you can just Mock RejectResult (returned from IsRejected) and is essentially the first and very obvious method above with only a small change to the RejectResult class (the addition of the For method). Here's the Email class and RejectResult class:
public class Email
{
    public RejectResult IsRejected
    {
        get{ return new RejectResult(true, new Reason("The reason this was rejected")); }
    }
}

public class RejectResult
{
    bool _rejected;
    Reason _reason;

    public RejectResult(bool rejected, Reason reason)
    {        
        this._rejected = rejected;
        this._reason = reason;
    }

    public bool For(out Reason reason)
    {
        reason = this._reason;
        return this._rejected;
    }
}
I love code that just explains itself without the need for copious comments!