On internal quality

I was asked by attendees at my VTM talk on test-driven development a small collection of questions on a similar theme, which I’ll summarise here.

  • How do I do TDD when my boss doesn’t want me to?
  • What do I do when my boss wants me to ship untested prototype code?
  • Can you give me rhetoric to convince my boss that I should be doing TDD?

I believe that I can adequately present these questions as facets of the following root issue, and then blabber away endlessly about that one instead.

I have externalised the obligation I have to myself to write software that makes me proud by delegating it up to my boss. Can you help me to sleep at night?

The fundamental problem here is one of cognitive dissonance. Belief: I, as a software engineer, believe that process/technique/tool is good for writing software. Introspection: I observe that I am not using said process/technique/tool. Justifications: it must be that my boss doesn’t hold my output to the same quality standard that I do. It must be that my deadlines and ever-shifting requirements don’t give me time to do things properly. It must be that this company doesn’t want great software engineers.

No. I have learned the hard way that such justifications are false. The only person responsible for how you write code is you. If you work on a team, or are handing your source code over to some other stakeholder (which means you’re working on a team), then you may have style guides that limit what source code you write; but ultimately how you get there is solely down to you.

You may be thinking “but I asked my manager for time to implement TDD/CI/buzzword-compliance on my current project and she said no. Also, by the way, you’re an arsehole for being so rude and presuming to know what I think.” Let me address the latter point first: suck it. Now onto the first point.

Your manager was correct to turn down your request for time on the project. Your customer doesn’t want unit tests, or comment documentation, or build server output. Your customer wants working features. The project exists to satisfy the user’s needs. Therefore the time allocated on the project should be dedicated to making working features.

Now it turns out that doing TDD is one way to write code that leads to working features, so what you should have done was to agree to the version of the product plan where you make working features, and then done TDD anyway. Where this all started going wrong was not when your boss turned down your request, but when you asked your boss in the first place. It turns out that you both have the same goal—making a good product—but that you have different views on the process and different motivators. By asking your boss how to write code you gave her permission to micro-manage you, then got frustrated when she did, and decided that the problem was all her fault.

I’ve seen this failure mode quite a few times now. As one example, I worked in a company where there was an ingrained antagonism between the product managers (who clearly just don’t get that software is a craft and a labour of love) and the programmers (who clearly just don’t get that we work in a competitive marketplace and would rewrite the product from scratch every day if they could).

As is common (and often correct) in our industry, the product managers owned the product requirements. As should be welcomed when we care about the products we make, the programmers were invited to criticise the requirements for the projects they were working on. Due to this antagonistic culture, the programmers would typically stuff “engineering requirements” onto the project, which were things like rewriting components from scratch, epic refactorings, or setting up new developer workflows.

Requirements were, as far as I could tell, prioritised based on how much revenue they would attract (i.e. new customers), how much revenue they would protect (retained customers) and how much they would cost. As you can probably already guess, so-called engineering requirements don’t protect any revenue, they don’t generate any revenue, and they do take time and money to implement, so they would inevitably get dropped or indefinitely postponed.

The moral of that little story is this: it is folly to try and express development processes and methodologies as business requirements, because they fail at being business requirements. It’s the equivalent of a taxi driver asking the customer to pay a $100,000 fare for one journey because he wants to switch to automatic transmission for the next journey and that means buying a new vehicle. The customer cares about being driven somewhere, and doesn’t care about how the driver operates the vehicle as long as the journey is bug-free. He certainly doesn’t intend to pay for the driver to select a particular mode of operation: if the driver wants to use automatic transmission, the driver should just get on and do that.

So what I’m saying is this: it’s not up to your boss, your customer or your project manager to choose how you write software. It’s up to you to choose how you write software, and as long as the approach you take leads to working software that delights your customers, the rest of the stakeholders won’t mind what you’re doing “under the hood”. My good friend and long-lost son @bmf once said “don’t let them see you making it.” I would go further: don’t let them know it was made. Let them use exciting, compelling software: the magic is your business. If you want to make the magic in one particular way, that would let you take additional pride in your creation, that’s entirely your call.

An example of unit testing working for me

Some specific feedback I was given regarding my unit testing talk at VTM: iPhone fall conference was that the talk was short on real-world application of unit testing. That statement is definitely true, and it’s unfortunate that I didn’t meet the attendee’s expectations for the talk. I hope to address that deficiency here, by showing you a real bug that I really fixed via unit testing.

The code is in the still-MIA Rehearsals app. I started developing Rehearsals before I was a full believer in test-driven development, so there were places in the app where I could go back and add tests to existing code. This is actually a pretty useful displacement activity – if you get bored of doing the real code, you can switch to writing the tests. This is enough of a change of pace (for me) to provide a welcome distraction, and still makes the product better.

So, in order to understand the problem, I need to understand the requirements. I’m a fan of the user story approach to requirements documentation, and so Rehearsals has the following story written on a 5×3 index card:

A musician can rate tunes in her tunebook, by giving it a number of stars between 1 and 5. Tunes that the musician has not rated have no star value.

As is often the case, there are other stories that build from this one (any such collection is called an “epic”), but I can concentrate on implementing this user story for now. One feature in Rehearsals is an AppleScript interface, so clearly if the musician can rate a tune, she must be able to do it via AppleScript. Glossing over the vagiaries of the Cocoa Scripting mechanism, I define a property scriptRating on the tune that had the following KVC-compliant accessors:

- (NSNumber *)scriptRating {
    if (!self.rating) {
        return [NSNumber numberWithInteger: 0];
    }
    return self.rating;
}

- (void)setScriptRating: (NSNumber *)newRating {
    NSInteger val = [newRating integerValue];
    if (val < 0 || val > 5) {
        //out of range, tell AppleScript
        NSScriptCommand *cmd = [NSScriptCommand currentCommand];
       [cmd setScriptErrorNumber: TORatingOutOfRangeError];
        //note that AppleScript is never localised
        [cmd setScriptErrorString: @"Rating must be between 0 and 5."];
    }
    else {
        self.rating = newRating;
    }
}

(No prizes for having already found the bug – it’s really lame). So testing that the getter works is ultra-simple: I can just -setRating: and verify that -scriptRating returns the same value. You may think this is not worth doing, but as this functionality is using indirect access via a different property I want to make sure I never break the connection. I decided that a single test would be sufficient (tune is an unconfigured tune object created in -setUp):

- (void)testTuneRatingIsSeenInAppleScript {
    tune.rating = [NSNumber numberWithInteger: 3];
    STAssertEquals([tune.scriptRating integerValue], 3,
                   @"Tune rating should be visible to AppleScript");
}

Simples. Now how do I test the setter? Well, of course I can just set the value and see whether it sticks, that’s the easy bit. But there’s also this bit about being able to get an error into AppleScript if the user tries to set an out of range error. That seems like really useful functionality, because otherwise the app would accept the wrong value and give up later when the persistent store gets saved. So it’d be useful to have a fake script command object that lets me see whether I’m setting an error on it. That’s easy to do:

@interface FakeScriptCommand : NSScriptCommand
{
    NSString *scriptErrorString;
    NSInteger scriptErrorNumber;
}
@property (nonatomic, copy) NSString *scriptErrorString;
@property (nonatomic, assign) NSInteger scriptErrorNumber;
@end

@implementation FakeScriptCommand
@synthesize scriptErrorString;
@synthesize scriptErrorNumber;
- (void) dealloc {...}
@end

OK, but how do I use this object in my test? The tune class sets an error on +[NSScriptCommand currentScriptCommand], which is probably returning an object defined in a static variable in the Foundation framework. I can’t provide a setter +setCurrentScriptCommand:, because I can’t get to the innards of Foundation to set the variable. I could change or swizzle the implementation of +currentScriptCommand in a category, but that has the potential to break other tests if they’re not expecting me to have broken Foundation.

So the solution I went for is to insert a “fulcrum” if you will: a point where the code changes from asking for a script command to being told about a script command:

- (void)setScriptRating: (NSNumber *)newRating {
    [self setRating: newRating fromScriptCommand: [NSScriptCommand currentScriptCommand]];
}

- (void)setRating: (NSNumber *)newRating fromScriptCommand: (NSScriptCommand *)cmd {
    NSInteger val = [newRating integerValue];
    if (val < 0 || val > 5) {
       [cmd setScriptErrorNumber: TORatingOutOfRangeError];
        //note that AppleScript is never localised
        [cmd setScriptErrorString: @"Rating must be between 0 and 5."];
    }
    else {
        self.rating = newRating;
    }
}

This is the Extract Method refactoring pattern. Now it’s possible to test -setRating:fromScriptCommand: without depending on the environment, and be fairly confident that if that works properly, -setScriptRating: should work properly too. I’ll do it:

- (void)testAppleScriptGetsErrorsWhenRatingSetTooLow {
    FakeScriptCommand *cmd = [[FakeScriptCommand alloc] init];
    [tune setRating: [NSNumber numberWithInteger: 0]
  fromScriptCommand: cmd];
    STAssertEquals([cmd scriptErrorNumber], TIRatingOutOfRangeError,
                   @"Should get a rating out of range error");
    STAssertNotNil([cmd scriptErrorString], @"Should report error to the user");
}

Oh-oh, my test failed. Why’s that? I’m sure you’ve spotted it now: the user is supposed to be restricted to values between 1 and 5, but the method is actually testing for the range 0 to 5. I could fix that quickly by changing the value in the test, but I’ll take this opportunity to remove the magic numbers from the code too (though you’ll notice I haven’t done that with the error string yet, I still need to do some more refactoring):

- (void)setRating: (NSNumber *)newRating fromScriptCommand: (NSScriptCommand *)command {
    if (![self validateValue: &newRating forKey: @"rating" error: NULL]) {
        [command setScriptErrorNumber: TIRatingOutOfRangeError];
        //note that AppleScript is never localised
        [command setScriptErrorString: @"Rating must be a number between 1 and 5."];
    }
    else {
        self.rating = newRating;
    }
}

OK, so now my test passes. Great. But that’s not what’s important: what’s important is that I found and fixed a bug by thinking about how my code is used. And that’s what unit testing is for. Some people will say they get a bad taste in their mouths when they see that I redesigned the method just so that it could be used in a test framework. To those people I will repeat the following: I found and fixed a bug.