DaedTech

Stories about Software

By

Walking the Line between Fanboy and Luddite

Are Unit Tests Overused?

I was reading this blog post by Andrew Hunter recently where he poses the question as to whether or not unit tests are overused. The core of his argument is a rather nuanced one that seems to hinge on two main points of wariness:

  1. Developers may view unit tests as a golden hammer and ignore other important forms of verification such as integration tests
  2. Unit tests that are too finely grained cause maintenance problems by breaking encapsulation.

I think the former is a great point, but am not really sold on the latter since I’d argue that the thing to do be to create a design where you didn’t have this problem (i.e. the problem isn’t with the amount of tests written, but with the design of the system).  But I don’t have any serious qualms with the point of the article, and I do like reading things like this from time to time as an ardent unit test proponent because the second you stop questioning the merit of your own practices is the same second you become the “because I’ve always done it that way, so get off my lawn” guy. Once I finished reading the article and scrolled down to the comments, however, I noticed an interesting trend.

From the Peanut Gallery

There were a total of 11 comments on the article. One of them was a complete non sequitur (probably SPAM), so let’s shave the ranks down to 10 comments, which makes the math easier anyway for percents. Four readers agreed with the author’s nuanced “we’re overdoing it” take, four seemed to take issue in favor of unit testing and two were essentially rants against unit testing that seemed to mistake the author’s questioning of the extent to which unit tests should be taken for sympathy with the position that they aren’t necessary. And thus in the percentage of these blog reading developers (which many would argue are the most informed developers) 40% defend TDD/Unit testing wholesale, 40% are generally amenable to it but think that some take it too far, and 20% are (angrily) opposed to the practice.

Of the 20% demographic, the percentage of them that have experience writing unit tests is 0, taking them at face value. One says “In over 10 years of development I can count the number of unit tests I’ve written on one hand,” and another says “I event[sic] tried to learn how TDD could help me.” Of the others, it appears that 100% of them have experience writing unit tests, though with some it is not as clear as others. None of them cops to having zero experience the way the detractors do. So among people with experience writing unit tests, there is a 50/50 split as to whether TDD practitioners have it right or whether they should write fewer unit tests and more integration tests. Among people with no experience writing unit tests, there is a 100/0 split as to whether or not writing unit tests is stupid.

That’s all just by the numbers, but if you actually delve into the ‘logic’ fueling the anti-testing posts, it’s garden variety fallacy. One argument takes the form of a simple false syllogism, “Guy X did TDD and guy X sucked, therefore TDD sucks.” The other takes the form of argument from ignorance, “I tried to learn TDD and didn’t like it/failed at it, ergo there is no benefit to it.” (I say failed because in spite of its brevity the post contains a fundamental misunderstanding of the ‘rules’ of TDD and so there was either a failure to understand or the argument is also a strawman).

To play dime store psychologist for a moment, it seems pretty likely to me that statements/opinions like these are attempts to rationalize one’s choices and that the posters protest too much, methinks — angry/exasperated disparaging of automated testing by those who have never tried it is likely hiding a bit of internal insecurity that the overwhelming consensus in the developer world is right and they are wrong. After all, this blog post and the subsequent comments are sort of like a group of chefs debating whether something would taste better with two teaspoons of sugar or three when along comes an admitted non-chef who says “I hate sugar because this guy I don’t like puts it in his coffee and I’ve never eaten it anyway, so it can’t be any good.” The chefs probably look at him with a suffering, bemused expression and give out with an “Aaannnnywho…”

Skeptic or Luddite, Informed or Fanboy?

Another way that one might describe this situation and pass judgment on both the development industry as a whole and these commenters too is to label them the developer equivalent of Luddites. After all, the debate as to the merits of unit testing is widely considered over and most of those who don’t do it say things like “I’d like to start” or they make up weird excuses. Unit testing is The Way Forward (caps intentional) and commenters like these are standing around, wishing for a simpler time where their approach was valued and less was asked of them.

But is that really fair? I mean, if you read my posts, my opinion on the merits of TDD and automated verification in general is no secret, but there’s an interesting issue at play here. Are people with attitudes like this really Luddites, or are they just conservative/skeptic types providing a counterpoint to the early-adopter types only ever interested in the new hotness of the week. You know the types that I mean – the ones who think TDD was so last year and BDD was so last month, and now it’s really all about YDD, which is so new and hot that nobody, including those doing it, knows what the Y stands for yet.

So in drawing a contrast between two roughly described archetypes, how are we to distinguish between “this might be a blip or passing fad” and “this seems to be a new way of doing things that has real value?” Walk too far on one side of the line and you risk being left behind or not taken seriously (and subsequently leaving angry justifications in blog comments) and walk too far on the other side of the line and you’ll engage in counterproductive thrashing and tilting at windmills in your approach, becoming a jack of all new trades while mastering none. Here are some things that I do personally in my attempt to walk this fine line, and would offer as advice to you if you’re interested:

  1. Pick out some successful developers/bloggers/authors/experts that you admire/respect and scan their opinions on things, when offered. It may seem a little “follow the herd,” but I view it as like asking friends for movie recommendations without going to see every movie that comes out.
  2. Don’t adopt any new practice/approach/etc unless you can articulate a problem that it solves for you.
  3. Limit your adoption bandwidth: if you’ve decided to adopt a new persistence model such as a NoSQL alternative to RDBMS, you might not also want to do it a new language (assuming that you’re doing this on someone else’s dime, anyway)
  4. Let others kick the tires a bit and then try it out. This gives you a bit of a bet hedge on whether something will fizzle and die before you commit to it.
  5. If you decide not to adopt something that seems new and hot, keep your finger on the pulse of it and read about its progress and the pains and joys of its adopters. It is possible not to adopt something without avoiding (or refusing to learn about) it.
  6. If you don’t see the value in something or you simply don’t have time/interest in it, don’t presume to think it has no value.
  7. Go to shows/conferences/talks/etc to see what all the fuss is about. This is a low-impact/low-commitment way to see what the fuss is about.

If you have suggestions for how to approach this balancing act, I’d be interested to hear them as well. Because while I think there’s definitely some margin for error, it’s important, if not easy, to avoid being either the person whose life is a never-ending string of partially completed projects and the person who has had the same year of COBOL programming experience for each of the past 30 years.

By

FeedPaper is Born!

As of Thursday, 11/02/12, a baby ASP MVC project named FeedPaper (later to be feedpapyr.us) has arrived. It is healthy and weights 0 pounds and 0 ounces. Both it and I are doing well, and while it does not yet have much in the way of functionality, it does have its father’s logo and some functional unit tests. (As an aside and leaving this strained metaphor, I’ve never really understood why the weight of babies is announced to me as if someone were trying to sell me a ham — what do I care how much anyone weighs? It’s always tempting for me to respond by saying, “big deal – I totally weigh more than that.”)

Anyway, you can find the source for it on Github. As I mentioned in a previous post, the main thing that I’m interested in is getting this thing up and running, so I’m happy to accept pull requests, suggestions, help, and anything else you’re willing to offer. The plan is to get it going as a website and then perhaps later port the presentation portion of it to phone/tablet implementations as well. But, no sense putting the cart before the horse — I have to figure out ASP MVC 4 first.

So, look for sporadic work on this when I have time and feel like tinkering and am not working on home automation with Java and MongoDB. I will also make posts here and there about lessons I learn as I ham-fist my way through it, talking about my experiences with the frameworks and toolsets involved. Also, in general, I’m looking for the best options for hosting the site, so suggestions are welcome (should I try out Azure, go a more traditional route, etc).

Cheers!

By

The Developer Incentive Snakepit

Getting Incentives Wrong

A while ago I was talking to a friend of mine, and he told me that developers in his shop get monetary bonuses for absorbing additional scope in a “waterfall” project. (I use quotes because I don’t think that “waterfall” is actually a thing — I think it’s just collective procrastination followed by iterative development) . That sounds reasonable and harmless at first. Developers get paid more money if they are able to adapt successfully to late-breaking changes without pushing back the delivery date. Fair enough…. or is it?

Image credit to Renaud d’Avout d’Auerstaedt via wikimedia commons.

Let’s digress for a moment.  In colonial India, the ruling British got tired of all of the cobras hanging around and being angry and poisonous and whatnot, so they concocted a scheme to address this problem. Reasoning that more dead cobras meant fewer living cobras, they began to offer a reward/bounty for any cobra corpses brought in to them by locals. As some dead cobras started coming in, circumstances initially improved, but after a while the number of dead cobras being brought in exploded even though the number of living cobras actually seemed to increase a little as well.  How is this possible?  The British discovered that Indian locals were (cleverly) breeding cobras that they could kill and thus cash in on the reward. Incensed, the British immediately discontinued the program. The cobra breeders, who had no more use for the things, promptly discarded them, resulting in an enormous increase in the local, wild cobra population. This is an iconic example of what has come to be called “The Law of Unintended Consequences“, which is a catch-all for describing situations where an incentive-based plan has an effect other than the desired outcome, be it oblique or directly counter. The Cobra Effect is an example of the latter.

Going back to the “money for changes absorbed” incentive plan, let’s consider if there might be the potential for “cobra breeders” to start gaming the system or perhaps less cynical, subconscious games that might go on. Here are some that I can think of:

  1. Change requests arise when the actual requirements differ from those defined initially, so developers have a vested interest in initial requirements being wrong or incomplete.
  2. Change requests will happen if the marketing/UX/PM/etc stakeholders don’t communicate requirements clearly to developers, so developers have a vested interest in avoiding these stakeholders.
  3. Things reported as bugs/defects/issues require changes to the software, which developers have to fix for free, but if those same items were, for some reason, re-classified as change requests, developers would make more money.
  4. The concept of “change request” doesn’t exist in agile development methodologies since any requirement is considered a change request, and thus developers would lose money if a different development methodology were adopted.

So now, putting our more cynical hats on, let’s consider what kind of outcomes these rational interests will probably lead to:

  1. Developers are actively obstructionist during the “requirements phase”.
  2. Developers avoid or are outright hostile toward their stakeholders.
  3. Developers battle endlessly with QA and project management, refusing to accept any responsibility for problems and saying things like “there was nothing in the functional spec that said it shouldn’t crash when you click that button!”
  4. Developers actively resist changes that would benefit the business, favoring rigidity over flexibility.

Think of the irony of item (4) in this scenario.  The ostensible goal of this whole process is to reward the development group for being more flexible when it comes to meeting the business demands.  And yet when incentives are created with the intention of promoting that flexibility, they actually have the effect of reducing it.  The managers of this department are the colonial British and the cash for absorbed change requests is the cash for dead cobras.  By rewarding the devs for dead cobras instead of fewer cobras, you wind up with more cobras; the developers’ goal isn’t more flexibility but more satisfied change requests, and the best way to achieve this goal is to generate software that needs a lot of changes as far as the business is concerned.  It’s an example of the kind of gerrymandering and sandbagging that I alluded to in an earlier post and it makes me wonder how many apparently absurd developer behaviors might be caused by nonsensical or misguided incentive structures triggering developers to game the system.

So what would be a better approach?  I mean with these Cobra Effect situations, hindsight is going to be 20/20.  It’s easy for us to say that it’s stupid to pay people for dead cobras, and it was only after ruminating on the idea for a while that the relationship between certain difficulties of the group in question and the incentive structure occurred to me.  These ideas seem generally reasonable on their faces and people who second guess them might be accused of arm-chair quarterbacking.

Get Rid of Cobras with Better Incentives

I would say that the most important thing is to align incentives as closely as possible with goals and to avoid rewarding process adherence.  For example, with this change request absorbed incentive, what is the actual goal, and what is being rewarded?  The actual reward is easy, particularly if you consider it obtusely (and you should): developers are rewarded when actual requirements are satisfied on time in spite of not matching the original requirements.  There’s nothing in there about flexibility or business needs — just two simple criteria: bad original requirements and on-time shipping.  I think there’s value in stating the incentive simply (obtusely) because it tends to strip out our natural, mile-a-minute deductions.  Try it on the British Empire with its cobra problem:  more cobra corpses means more money.  There’s nothing that says the cobra corpses have to be local or that the number of cobras in the area has to decrease.  That’s the incentive provider being cute and clever and reasoning that one is the logical outcome of the other.

Flexibility image credit to “RDECOM” via wikimedia commons.

Going back to the bad software incentives, the important thing is to pull back and consider larger goals.  Why would a company want change requests absorbed?  Probably because absorbed change requests (while still meeting a schedule) are a symptom (not a logical outcome, mind you) of flexibility.  Aha, so flexibility is the goal, right?  Well, no, I would argue.  Flexibility is a means rather than an end.  I mean, outside of a circus contortionist exhibit, nobody gets paid simply to be flexible.  Flexibility is a symptom of another trend, and I suspect that’s where we’ll find our answer.

Why would a company want to be flexible when it comes to putting out software?  Maybe they have a requirement from the marketing department stating that they want to cut a sort of hip, latest-and-greatest feel, so the GUI has to constantly be updated with whatever latest user experience/design trends are coming from Apple or whoever is currently the Georgio Armani of application design.  Maybe they’re starting on a release now and they know their competition is coming out with something in 3 months that they’re going to want to mimic in the next release, but they don’t yet know what that thing is.  Maybe they just haven’t had any resources available to flesh out more than a few wireframes for a few screens but don’t want all progress halted, “waterfall” style, until every last detail of every last screen is hammered out in IRS Tax Code-like detail.

Aha!  Now we’re talking about actual business goals rather than slices of process that someone thinks will help the goals be achieved.  Why not reward the developers (and QA, marketing, UX, etc as well) for those business goals being met rather than for adherence to some smaller process?  Sadly, I think the answer is a command and control style of hierarchical management that often seeks to justify positions with fiefdoms of responsibility and opacity.  In other words, many organizations will have a CEO state these business goals and then hand down a mandate of “software should be flexible” to some VP, who in turn hands down a mandate of “bonuses for change requests absorbed” to the manager of the software group or some such thing.  It is vital to resist that structure as much as possible since providing an incentive structure divorced from broader goals practically ensures that the prospective recipients of the structure care nothing whatsoever for anything other than gaming the system in their own favor.  And in some cases, such as our case, this leads to open and predictable conflicts with other groups that have (possibly directly contradicting) incentive structures of their own.  As an extreme example, imagine a tech group where the QA team gets money for each bug they find and the developers lose money for the same.  A good way to ensure quality… or a good way to ensure fistfights in your halls?

Take-Away

Why keep things so simple and not fan out the deductive thinking about incentives, particularly in the software world?  Well, software people by nature are knowledge workers that are paid to use well-developed, creative problem-solving skills.  Incentives that work on assembly line workers such as “more money for more holes drilled per hour” are more likely to backfire when applied to people who make their living inventing ways to automate, enhance and optimize processes.  Software people will become quickly adept at gaming your system because it’s what you pay them to do.  If you reward them for process, they’ll automate to maximize the reward, whether or not it’s good for the business.  But if you reward them for goals met, they will apply their acute problem solving skills to devising the best process for solving the problem — quite possibly better than the one you advise them to follow.

If you’re in a position to introduce incentives, think carefully about the incentives that you introduce and how closely they mirror your actual goals.  Resist the impulse to reward people for following some process that you assume is the best one.  Resist the impulse to get clever and think “If A, then B, then C, then D, so I’ll reward people for D in order to get A.”  That’s the sort of thinking that leads to people on your team dropping cobras in your lunch bag.  Metaphorical cobras.  And sometimes real cobras.  If you work with murderers.

By

TDD For Breaking Problems Apart 3: Finishing Up

Last time, we left off with a bowling score calculator that handled basic score calculation with the exception of double strikes and the tenth frame. Here is the code as of right now for both classes:

[TestClass]
public class BowlingTest
{
    [TestClass]
    public class Constructor
    {

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_Score_To_Zero()
        {
            var scoreCalculator = new BowlingScoreCalculator();

            Assert.AreEqual(0, scoreCalculator.Score);
        }
    }

    [TestClass]
    public class BowlFrame
    {
        private static BowlingScoreCalculator Target { get; set; }

        [TestInitialize()]
        public void BeforeEachTest()
        {
            Target = new BowlingScoreCalculator();
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void With_Throws_0_And_1_Results_In_Score_1()
        {
            var frame = new Frame(0, 1);
            Target.BowlFrame(frame);

            Assert.AreEqual(frame.FirstThrow + frame.SecondThrow, Target.Score);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void With_Throws_2_And_3_Results_In_Score_5()
        {
            var frame = new Frame(2, 3);
            Target.BowlFrame(frame);

            Assert.AreEqual(frame.FirstThrow + frame.SecondThrow, Target.Score);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Sets_Score_To_2_After_2_Frames_With_Score_Of_1_Each()
        {
            var frame = new Frame(1, 0);
            Target.BowlFrame(frame);
            Target.BowlFrame(frame);

            Assert.AreEqual(frame.Total + frame.Total, Target.Score);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Sets_Score_To_Twenty_After_Spare_Then_Five_Then_Zero()
        {
            var firstFrame = new Frame(9, 1);
            var secondFrame = new Frame(5, 0);

            Target.BowlFrame(firstFrame);
            Target.BowlFrame(secondFrame);

            Assert.AreEqual(20, Target.Score);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Sets_Score_To_25_After_Strike_Then_Five_Five()
        {
            var firstFrame = new Frame(10, 0);
            var secondFrame = new Frame(6, 4);

            Target.BowlFrame(firstFrame);
            Target.BowlFrame(secondFrame);

            Assert.AreEqual(30, Target.Score);
        }
    }
}

public class BowlingScoreCalculator
{
    private readonly Frame[] _frames = new Frame[10];

    private int _currentFrame;

    private Frame LastFrame { get { return _frames[_currentFrame - 1]; } }

    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        AddMarkBonuses(frame);

        Score += frame.Total;
        _frames[_currentFrame++] = frame;
    }

    private void AddMarkBonuses(Frame frame)
    {
        if (WasLastFrameAStrike()) Score += frame.Total;
        else if (WasLastFrameASpare()) Score += frame.FirstThrow;
    }

    private bool WasLastFrameAStrike()
    {
        return _currentFrame > 0 && LastFrame.IsStrike;
    }
    private bool WasLastFrameASpare()
    {
        return _currentFrame > 0 && LastFrame.IsSpare;
    }
}
[TestClass]
public class FrameTest
{
    [TestClass]
    public class Constructor
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_FirstThrow_To_Passed_In_Value()
        {
            var frame = new Frame(1, 0);

            Assert.AreEqual(1, frame.FirstThrow);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_SecondThrow_To_Passed_In_Value()
        {
            var frame = new Frame(0, 1);

            Assert.AreEqual(1, frame.SecondThrow);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Throws_Exception_On_Negative_Argument()
        {
            ExtendedAssert.Throws(() => new Frame(-1, 0));
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Throws_Exception_On_Score_Of_11()
        {
            ExtendedAssert.Throws(() => new Frame(Frame.Mark, 1));
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Initializes_Total_To_FirstThrow_Plus_SecondThrow()
        {
            const int firstThrow = 4;
            const int secondThrow = 3;

            Assert.AreEqual(firstThrow + secondThrow, new Frame(firstThrow, secondThrow).Total);
        }
    }

    [TestClass]
    public class IsStrike
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Returns_True_When_Frame_Has_10_For_First_Throw()
        {
            var frame = new Frame(10, 0);
            Assert.IsTrue(frame.IsStrike);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Returns_False_When_Frame_Does_Not_Have_10_For_First_Throw()
        {
            var frame = new Frame(0, 2);
            Assert.IsFalse(frame.IsStrike);
        }
    }

    [TestClass]
    public class IsSpare
    {
        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Returns_True_When_Frame_Totals_Mark()
        {
            var frame = new Frame(4, 6);

            Assert.IsTrue(frame.IsSpare);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Returns_False_When_Frame_Does_Not_Total_10()
        {
            var frame = new Frame(0, 9);
            Assert.IsFalse(frame.IsSpare);
        }

        [TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
        public void Returns_False_When_Frame_Is_Strike()
        {
            var frame = new Frame(10, 0);
            Assert.IsFalse(frame.IsSpare);
        }
    }
}

public class Frame
{
    public class UnderflowException : Exception { }

    public class OverflowException : Exception { }

    public const int Mark = 10;

    public int FirstThrow { get; private set; }

    public int SecondThrow { get; private set; }

    public int Total { get { return FirstThrow + SecondThrow; } }

    public bool IsStrike { get { return FirstThrow == Frame.Mark; } }

    public bool IsSpare { get { return !IsStrike && Total == Frame.Mark; } }

    public Frame(int firstThrow, int secondThrow)
    {
        if (firstThrow < 0 || secondThrow < 0)
            throw new UnderflowException();
        if (firstThrow + secondThrow > Mark)
            throw new OverflowException();

        FirstThrow = firstThrow;
        SecondThrow = secondThrow;
    }
}

Without further ado, let’s get back to work. The first thing I’d like to do is actually a refactor. I think it would be more expressive when creating strikes to use a static property, Frame.Strike, rather than new Frame(10, 0). Since the strike is completely specific in nature and a named case, I think this approach makes sense. So the first thing that I’m going to do is test that it returns a frame where IsStrike is true:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Returns_Frame_With_Is_Strike_True()
{
    Assert.IsTrue(Frame.Strike.IsStrike);
}

(This is actually what the test looked like after two red-green-refactors, since the first one was just to define Frame.Strike). At this point, I now have a static property that I can use and I’m going to find everywhere in my score calculator and frame test classes that I queued up a strike and use that instead as part of this refactor cycle. While I’m at it, I also demote the visibility of Frame.Mark, since I realize I should have done that a while ago. The Mark constant isn’t needed outside of Frame since Frame is now expressive with IsStrike, IsSpare and Total. Strictly speaking, I should conceive of some test to write that will fail if Mark is visible outside of the class, but I try to be pragmatic, and that’s a screwy test to have and persist.

Now, let’s get down to brass tacks and fix the double strike issue. If I bowl a strike in the first frame, another in the second frame, and then a 9 in the third frame, my total score should be 57 in the third (29+19+9). Let’s write such a test:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Sets_Score_To_57_After_Two_Strikes_And_A_Nine()
{
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(new Frame(9, 0));

    Assert.AreEqual(57, Target.Score);
}

How to get this to pass… well, I’ll just tack on an extra frame.FirstThrow if the last two were strikes:

private void AddMarkBonuses(Frame frame)
{
    if (_currentFrame > 1 && LastFrame.IsStrike && _frames[_currentFrame - 2].IsStrike)
        Score += frame.Total;

    if (WasLastFrameAStrike()) 
        Score += frame.Total;
    else if (WasLastFrameASpare()) 
        Score += frame.FirstThrow;
}

… and NCrunch gives me green. Now, let’s make the class a little nicer to look at:

public class BowlingScoreCalculator
{
    private readonly Frame[] _frames = new Frame[10];

    private int _currentFrame;

    private Frame LastFrame { get { return _frames[_currentFrame - 1]; } }

    private Frame TwoFramesAgo { get { return _frames[_currentFrame - 2]; } }

    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        AddMarkBonuses(frame);

        Score += frame.Total;
        _frames[_currentFrame++] = frame;
    }

    private void AddMarkBonuses(Frame frame)
    {
        if (WereLastTwoFramesStrikes())
            Score += 2 * frame.Total;
        else if (WasLastFrameAStrike()) 
            Score += frame.Total;
        else if (WasLastFrameASpare()) 
            Score += frame.FirstThrow;
    }

    private bool WereLastTwoFramesStrikes()
    {
        return WasLastFrameAStrike() && _currentFrame > 1 && TwoFramesAgo.IsStrike;
    }

    private bool WasLastFrameAStrike()
    {
        return _currentFrame > 0 && LastFrame.IsStrike;
    }
    private bool WasLastFrameASpare()
    {
        return _currentFrame > 0 && LastFrame.IsSpare;
    }
}

And, that’s that. Now we have to think about the 10th frame. This is going to be interesting because the 10th frame is completely different in concept than the other frames. The 10th frame’s total can range up to 30 instead of being capped at 10, and if you get a strike in the first frame or spare in the second frame, you get three throws instead of two. How to model this with what we have… add a new property to the frame class called “ThirdThrow”? That seems reasonable, but what if we populate the third throw when we’re not in the 10th frame? That’s no good — how can we know that a frame is a 10th frame? We’ll probably need a boolean property called IsTenthFrame… right?

Wrong! (At least in my opinion). That amounts to adding a flag that clients look at to know how to treat the object. If the flag is set to true, we treat it like one kind of object and if it’s set to false, we treat it like another kind. This is a code smell in my opinion — one that I think of as “polymorphism envy” or “poor man’s polymorphism”. This is a milder version of the kind you usually see which is some ObjectType enum that clients switch over. We don’t have that (yet) because we only have two values.

So if we’re contemplating a polymorphism envy approach, it stands to reason that maybe what we’re nibbling at is, well, actual polymorphism. Maybe we should have a TenthFrame class that derives from Frame and overrides important functionality. I don’t know that this is the solution, but TDD is about solving small problems incrementally, so let’s start down this path and see where it leads. We don’t need all of the answers this minute. The first thing to test is probably going to be that total is the sum of the three constructor arguments:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Initializes_Total_To_Sum_Of_Three_Throws()
{
    const int firstThrow = 1;
    const int secondThrow = 2;
    const int thirdThrow = 3;
    var frame = new TenthFrame(firstThrow, secondThrow, thirdThrow);

    Assert.AreEqual(firstThrow + secondThrow + thirdThrow, frame.Total);
}

As I wrote this test, two things didn’t compile. The first was the instantiation of the TenthFrame, which I solved by declaring it. The second was the Total property, which I solved by inheriting from Frame. That actually turned out to be an easier way to get a red test than declaring the property (and more productive toward our design). Then to get the test passing, the easiest thing to do was make Frame’s total virtual and override it in TenthFrame. So, pretty quickly we seem to be getting Total right:

public class TenthFrame : Frame
{
    public int ThirdThrow { get; private set; } 

    public override int Total { get { return base.Total + ThirdThrow; }  }

    public TenthFrame(int firstThrow, int secondThrow, int thirdThrow) : base(firstThrow, secondThrow)
    {
        ThirdThrow = thirdThrow;
    }
}

Now we need to start tweaking the business rules. Parent is going to throw an exception if we initialize frame 1 and 2 each to a strike, but that’s fine in the 10th frame. Here’s a failing test:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Does_Not_Throw_Exception_On_Two_Strikes()
{
    ExtendedAssert.DoesNotThrow(() => new TenthFrame(10, 10, 9));
}

To get this passing, I declare a default constructor in the base (to make the compiler happy) and have the new class’s constructor implement its own assignment logic to avoid the checks in parent that cause this failure. But now that simple assignment is restored, we need to implement our own rules, which will include throwing exceptions for throws greater than ten or less than zero, but it will also include oddballs like this that I don’t yet know how to describe:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Throws_Exception_For_5_10_5()
{
    ExtendedAssert.Throws(() => new TenthFrame(5, 10, 5));
}

This is another one of the real draws of TDD. I don’t know what to call this or how to categorize it, but I have an example, and that’s all I need to get started. I just have to make this pass:

public TenthFrame(int firstThrow, int secondThrow, int thirdThrow) 
{
    ValidateIndividualThrows(firstThrow, secondThrow, thirdThrow);

    if (firstThrow != Mark && secondThrow + firstThrow > Mark)
        throw new IllegalFrameException();

    FirstThrow = firstThrow;
    SecondThrow = secondThrow;
    ThirdThrow = thirdThrow;
}

I could have just tested for the specific literals in the test, but I didn’t feel the need to be that obtuse. You can really control your own destiny somewhat with “simplest thing to make the test pass”. IF you have no idea what direction the design should take, maybe you go that obtuse route. If you have some half-formed idea, as I do here, it’s fine to get a little more business-logic-y. I’m going to dial up another case that should fail because of the relationship between second and third frame and take if from there:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void Throws_For_10_5_10()
{
    ExtendedAssert.Throws(() => new TenthFrame(10, 5, 10));
}

Now I have the following production code to get it to pass:

public TenthFrame(int firstThrow, int secondThrow, int thirdThrow) 
{
    ValidateIndividualThrows(firstThrow, secondThrow, thirdThrow);

    if (firstThrow != Mark && secondThrow + firstThrow > Mark)
        throw new IllegalFrameException();
    if (secondThrow != Mark && thirdThrow + secondThrow > Mark)
        throw new IllegalFrameException();

    FirstThrow = firstThrow;
    SecondThrow = secondThrow;
    ThirdThrow = thirdThrow;
}

But, that’s getting a little fugly, so let’s refactor:

public TenthFrame(int firstThrow, int secondThrow, int thirdThrow) 
{
    ValidateIndividualThrows(firstThrow, secondThrow, thirdThrow);
    CheckConsecutiveThrows(firstThrow, secondThrow);
    CheckConsecutiveThrows(secondThrow, thirdThrow);

    FirstThrow = firstThrow;
    SecondThrow = secondThrow;
    ThirdThrow = thirdThrow;
}

private static void CheckConsecutiveThrows(int first, int second)
{
    if (first != Mark && first + second > Mark)
        throw new IllegalFrameException();
}

Ah, a business rule is starting to emerge. In general, if a throw is not a mark, then it and the subsequent throw can’t be greater than 10. Hey, come to think of it, that sounds right from my bowling experience. They only reset the pins if you knock ’em all down. Of course, we have one final rule to implement, which is that if the first and second throws don’t knock all the pins down, there is no third throw. I’ll leave that out, since it’s pretty easy.

Now the time has arrived for some integration testing. I found a site that has some example bowling scores, and I’m going to code one up:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void SampleGame_For_Mary()
{
    Target.BowlFrame(new Frame(9, 0));
    Target.BowlFrame(new Frame(3, 7));
    Target.BowlFrame(new Frame(6, 1));
    Target.BowlFrame(new Frame(3, 7));
    Target.BowlFrame(new Frame(8, 1));
    Target.BowlFrame(new Frame(5, 5));
    Target.BowlFrame(new Frame(0, 10));
    Target.BowlFrame(new Frame(8, 0));
    Target.BowlFrame(new Frame(7, 3));
    Target.BowlFrame(new TenthFrame(8, 2, 8));

    Assert.AreEqual(131, Target.Score);
}

If you’re following along, you’ll see green in NCrunch. Let’s try one more:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void SampleGame_For_Kim()
{
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(new Frame(3, 7));
    Target.BowlFrame(new Frame(6, 1));
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(Frame.Strike);
    Target.BowlFrame(new Frame(2, 8));
    Target.BowlFrame(new Frame(9, 0));
    Target.BowlFrame(new Frame(7, 3));
    Target.BowlFrame(new TenthFrame(10, 10, 10));

    Assert.AreEqual(193, Target.Score);
}

Oops. Red. So, what happened? Well, I went back through game, setting temporary asserts until I found that things went off the rails following the third strike in a row. I then looked in my class at the logic following the two strikes and realized it wasn’t quite right:

private void AddMarkBonuses(Frame frame)
{
    if (WereLastTwoFramesStrikes())
        Score += frame.Total + frame.FirstThrow;
    //Score += 2 * frame.Total;
    else if (WasLastFrameAStrike())
        Score += frame.Total;
    else if (WasLastFrameASpare())
        Score += frame.FirstThrow;
}

I commented out the mistake and put in the correct code, and the entire test suite went green, including the integration test for Kim. I think this is a good note to close on because the tests are all passing and I believe the calculator is functioning (here it is on gist if you want to check out the final product) but also because I think there’s a valuable point here.

TDD is a design methodology — not a guarantee of bug free code/comprehensive testing strategy. I experienced and even blogged about writing code for days using TDD and assembling all the parts and having it work flawlessly the first time, and that did happen. But I realized that even with that, the happy and happy-ish paths were the ones that worked. Here, I had a bowling calculator that made it through all individual scoring tests and even an entire non-trivial game going green before we teased out a case in smoke testing where it went red.

TDD will help you break problems into small pieces to solve (the whole point of this series of posts), ensure that your code is testable and thus loosely coupled and modular, ensure that you don’t push dirty mop water around the floor by breaking functionality that you had working before, and generally promote good code. But think about this — those are all productive programming concerns rather than testing concerns. You still need testers, you still need edge case unit tests once you’re done with TDD, and you still need integration/smoke tests. The fact that TDD produces a lot of tests that you can check in and continue to use is really just a bonus.

And it’s a bonus that keeps on giving. Because let’s say that you don’t agree with my decision to use inheritance for tenth frame or you think strike would be a more appropriate descriptor of a throw than of a frame. With all of my TDD test artifacts (and the integration tests) in place, you can rip my internal design to pieces without worrying that you’re going to break the functionality that this provides to clients. And that’s incredibly powerful for allowing fearless refactoring and maintenance of this code. So do it to break your problems apart and keep yourself moving and productive, and keep the tests around to make sure the code stays clean and trends toward improvement rather than rot.

Full Code

By

Hilarious Conditional Bloopers!

For this Friday, I thought I’d do something a little more lighthearted and, in the tradition of bad television (or Robot Chicken’s satire thereof) post some programming bloopers. These are actual things that I’ve personally seen in source code as opposed to some kind of specific sampling of CodeSOD from the Daily WTF. Doing this series of posts about Boolean algebra made me think conditional logic fails I’ve seen both recently and long in the past.

For each one, I’ve tried my best to give it a catchy name, an explanation of the problem, an example of what it translates to in simple English (i.e. why it doesn’t “read like well written prose”), and what it ought to look like. So, without further ado, here are the bloopers:

The Ingrown Branch

private int _numberOfMilkCartons;
private bool _amIOutOfMilk;

public void FigureOutWhatToDo()
{
    if(_numberOfMilkCartons != 12)
        _numberOfMilkCartons = 12;
}

I call this The Ingrown Branch because of what it does. It introduces a conditional — a fork in the road, if you will — and it winds up in the same spot no matter which branch you take. In conversational English, this says “if the number of milk cartons is not 12, make it 12”. While this doesn’t sound ridiculous in the same way that others here will, consider what’s being done. If x is equal to 12, well, then do nothing because x is equal to 12. Otherwise, if it’s not equal to 12, set it to 12. What would be a simpler way to do this?

private int _numberOfMilkCartons;
private bool _amIOutOfMilk;

public void FigureOutWhatToDo()
{
    _numberOfMilkCartons = 12;
}

The Tautology

private bool _amIOutOfMilk;

public void FigureOutWhatToDo()
{
    if(_amIOutOfMilk || !_amIOutOfMilk)
        GoToTheStore();
}

In conversational English, a tautology is something that is vacuously true or redundant. In logic, this is something that is always true, ipso facto, such as “A or NOT(A)”, for instance. In terms of conversational English, this is like saying “If I’m out of milk or if I’m not out of milk, I’m going to go buy some milk.” Why not drop the spurious conditionals and get to the point:

public void FigureOutWhatToDo()
{
    GoToTheStore();
}

The Contradiction

private bool _amIOutOfMilk;

public void FigureOutWhatToDo()
{
    if(_amIOutOfMilk == !_amIOutOfMilk)
        GoToTheStore();
}

The opposite of a tautology, a contradiction is something that is vacuously false, such as primitive type not being equal to itself. With instances like this and the tautology, I usually see more complex incarnations that are harder to spot or else I give the benefit of the doubt and assume that manipulation of a more complex conditional occurred in the past and the thing was accidentally left in this condition. But this doesn’t alter the fact that I have seen code like this and that, in plain English, this would translate to “If I’m both completely out of milk and I have some milk, I’m going to buy milk.” It’s mind-bending nonsense that would best be described as:

//Yep, nothing at all

The Double Negative

private bool _amINotOutOfMilk;

public void FigureOutWhatToDo()
{
    if(!_amINotOutOfMilk)
        GoToTheStore();
}

I realize that this may be largely a product of speaking English as a first language, since double (and more) negatives are acceptable in some other languages. But you have to look at code like this and think, did anyone read this to themselves? “If I it’s false that I’m not out of milk, I will go to the store.” Wat? Okay, so not out of milk means that you have it, so if it’s false that you’re not out of milk, it’s false that you have it, and you are out of milk… aha! Why didn’t you just say so:

public void FigureOutWhatToDo()
{
    if(_amIOutOfMilk)
        GoToTheStore();
}

Ifception

public void FigureOutWhatToDo()
{
    if(_amIOutOfMilk)
        if(_amIOutOfEggs)
            if(_amIOutOfBeer)
                GoToTheStore();
}

An if within an if within an if… (credit to Dan Martin for this term). This is another mind-bending way of writing things that is rather jarring to the reader of the code, like saying “If I’m out of milk if I’m out of eggs if I’m out of beer, then I’m going to the store.” Dude, wat? Oh, you mean “If you’re out of milk AND you’re out of eggs AND you’re out of beer, then you’re going to the store? Well, nice to see what your breakfast priorities are, but at least that doesn’t read like the mumblings of a lunatic.”

The Robot

public void FigureOutWhatToDo()
{
    if(_amIOutOfMilk == true)
        GoToTheStore();
}

Perhaps this is a little nitpicky, but this explicit formation of Boolean conditionals bothers me. “If it equals true that I am out of milk, I will go to the store” sounds like some robot helper from the Jetsons or one of those shows that features a preposterous token “genius” whose intelligence is conveyed by having him speak like some robot helper from the Jetsons. Why not “If I’m out of milk, I will go to the store?”

public void FigureOutWhatToDo()
{
    if(_amIOutOfMilk)
        GoToTheStore();
}

The Yoda

private Milk _theMilk;

public void FigureOutWhatToDo()
{
    if(null == _theMilk)
        GoToTheStore();
}

If program in C you do, sense this makes and a clever trick to avoid assignment instead of comparison errors this is. If program in C you don’t, annoying and indicative that you’re not adopting the thinking of the language you’re working this is. When you speak English and try to sound like a native speaker, you don’t say “If missing is the milk, go to the store”. You say “If the milk is missing, go to the store.”

public void FigureOutWhatToDo()
{
    if(_theMilk == null)
        GoToTheStore();
}

The Existential No-Op

public void FigureOutWhatToDo()
{
    if(_amIOutOfMilk)
    {
        //GoToTheStore();
    }
}

Or, see variations where the comment is replaced by “return;” or some other similar thing. This is a conditional where, true or false, you do nothing. It sort of makes you question the very nature of (its) existence. This is like me saying to you “If I’m out of milk…” When you wait patiently for a moment and say “yes…?” I then say “nothing — that was all.” What should this be replaced with? How about just deleting the whole bit of nonsense?

Growing Pains

public void FigureOutWhatToDo()
{
    if(_amIOutOfMilk && _amIOutOfEggs && _amIOutOfBeer && _amIOutOfCheese && _amIOutOfChips && _amIOutOfMilk)
        GoToTheStore();
}

See what’s going on here? This conditional is growing so unwieldy that you forget by the end of it that you already mentioned being out of milk again. “If I’m out of milk, eggs, beer and milk, I’m going to the store.” “You said milk twice.” “I like milk.” How about dividing it up a bit and saying “If I am out of staples and I’m out of snacks, then I’m going to the store.”

public void FigureOutWhatToDo()
{
    if(AmIOutOfStaples() && AmIOutOfSnacks())
        GoToTheStore();
}

private bool AmIOutOfSnacks()
{
    return _amIOutOfBeer && _amIOutOfChips;
}

private bool AmIOutOfStaples()
{
    return _amIOutOfMilk && _amIOutOfEggs && _amIOutOfCheese;
}

The Mad Scoper

public void FigureOutWhatToDo()
{
    if(((AmIOutOfStaples())) && (AmIOutOfSnacks()))
        GoToTheStore();
}

I think we’ve all seen one of these — someone on the team or in the group has a few too many cups of coffee and really goes to town on the old 9 and 0 keys. This is probably done to make sure that order of operations is being followed when you don’t understand the order of operations. Conversational equivalent? “If I am out of staples, and I mean staples and not whatever is coming next until I’m ready to talk about that and now I’m ready so I’m going to talk about that and that is snacks and not staples we’re not talking about staples anymore we’re talking about snacks which if I’m out of I’m also not going to the store, okay done.” Let’s dial it back to the last solution:

public void FigureOutWhatToDo()
{
    if(AmIOutOfStaples() && AmIOutOfSnacks())
        GoToTheStore();
}

The Fly Swallower (aka The Train Wreck)

public class OldWoman
{
    public Horse _horse = new Horse();

    public object WhatDidYouSwallow()
    {
        if (_horse != null && _horse.Cow != null && _horse.Cow.Hog != null && _horse.Cow.Hog.Dog != null &&
            _horse.Cow.Hog.Dog.Cat != null && _horse.Cow.Hog.Dog.Cat.Bird != null &&
            _horse.Cow.Hog.Dog.Cat.Bird.Spider != null &&
                _horse.Cow.Hog.Dog.Cat.Bird.Spider.Fly != null)
            return _horse.Cow.Hog.Dog.Cat.Bird.Spider.Fly;

        return null;
    }
}

This is formally known as design with violations of the Law of Demeter, but it’s easier just to think of it as a train wreck. But the name I’m giving it comes from a nursery rhyme, which is how this starts to sound in conversational English. “There was an old lady who if she swallowed a horse who if it swallowed a cow who if it swallowed a hog who if it swallowed a dog…” How should this sound? There’s no easy fix. You need a different object model.

And with that, I’ll conclude my fun Friday post. This is meant to be light-hearted and in jest, but I’d say there’s definitely a good bit of truth here. You may not agree entirely with my assessment, but I think we’d all be well served to do the occasional double check to make sure we’re not leaving conditional bloopers in the code for others to read, triggering pointing and laughter. If you have other names for these or other conditional bloopers to mention (or you think I’m completely off base with one of these) please feel free to chime in.

By the way, if you liked this post and you're new here, check out this page as a good place to start for more content that you might enjoy.