DaedTech

Stories about Software

Mar
9

By

What TDD Is and Is Not

Category: Language Agnostic Tags: , | 20 Comments

In my travels, I’ve heard a lot of people embrace test driven development (TDD) and I’ve heard a lot of people say that it isn’t for them. For the most part, it’s been my experience that people who say it isn’t for them have either never really, actually done it or haven’t done it enough to be good at it.

Lest you think I’m just being judgmental, here’s a post of mine from about 3 years ago, before I had truly taken the humbling plunge of forcing myself to follow the red-green-refactor discipline to the letter. My language here is typical of someone who buys into unit testing and even to TDD, but who hasn’t become facile enough with the process to avoid feeling the need to tinker with it and qualify the work. If you cut to the heart of what’s going on in my post there (and I say this hat in hand) it’s less “here’s my own special way of doing TDD that I think is actually superior” and more “I haven’t yet gotten good enough at TDD to use it everywhere and that’s rather frustrating.”

Stoplight

Not long after I wrote that post, I forced myself, on a project, to follow the discipline, to the letter, and I’ve never looked back after the initial pain of floundering during what I used to describe as the “prototyping stage” of my coding. I solved the ‘problem’ of TDD not being compatible with that prototyping by thinking my designs through more carefully up-front, solving only problems that actually exist, and essentially not needing it anymore. TDD wasn’t the problem for me when I wrote that post; the problem was that I wasn’t being efficient in my approach.

It’s been my experience that most of the criticism of TDD comes from people like me, years ago. These are people who either have never actually tried TDD or who have tried it for an amount of time too short to become proficient with it. I can rarely recall someone who became proficient with the practice one day saying, “you know what, enough of this — I don’t see the benefit.” For me this statement is hard to imagine because TDD shortens the feedback loop between implementing something and know if it works, and developers innately crave tight feedback loops.

And since the majority of criticism seems to come from those least familiar with the discipline, there are a lot of misconceptions and straw man arguments, as one might expect. So today, I’d like to offer some clarity by way of defining what TDD is and what it it isn’t.

What TDD Isn’t

I think it’s important that I first discuss what TDD is not. There are a lot of misconceptions that surround the test driven development approach to writing software, and there’s a pretty good chance that you’ve at least been exposed to them, if not slightly misled by them. Not only does this make it hard to understand how the practice works, but it may even lead you or your team to reject it for reasons that aren’t actually valid.

  1. First, TDD is not a replacement for user acceptance testing. Someone who practices it does not believe that it’s a valid substitute for running the application and making sure that it does what the requirements state that it needs to do.
  2. Classic TDD is also not comprehensive automated testing. The by-product of it is mainly unit tests, which are tests for finely grained pieces of code, such as classes and methods. Other kinds of automated tests, such as integration tests and end to end system tests involving databases or other external constructs will be a separate prong of your overall testing strategy.
  3. One thing that I frequently hear as an indictment of TDD is that it doesn’t encourage you to think through your design because you initially do the simplest thing to make tests pass. This is not accurate at all. It simply distributes the planning over the course of your development as you go rather than forcing it all to be done up front.
  4. Test driven development is not and does not claim to be any sort of load testing, concurrency testing, or anything else that you might put under the category of “smoke” or “stress” testing. The tests generated by TDD are not meant to test the behavior of your system under adverse conditions.
  5. Another common misconception is that TDD means that you write all tests for the system before you start writing your code. Frankly, doing so would be wildly impractical, and detractors who cite this impracticality as a reason not to do TDD are arguing against a straw man.
  6. TDD is not a quality assurance strategy. When your software department is contemplating new initiatives and dividing them up according to who will own then, TDD does not belong to the testing group.
  7. Detractors of TDD often point out that it doesn’t address corner cases in application or even class and method logic. That is true, but TDD doesn’t aim to address these. They belong with the other automated tests that will be written later.
  8. And, believe it or not, TDD is not primarily a testing activity. This is probably the hardest for people to wrap their heads around when learning the practice. But if you think about the acronym – test driven development, it is primarily development. The tests driving it are simply a characteristic of the development.

What TDD Is

Having gone over a series of things that TDD is not, hopefully I’ve cleared up some misconceptions and narrowed the field a bit. So let’s take a look at what TDD actually is. I should note here that the flavor of TDD that I’m addressing is “Classic,” triangulation-oriented TDD rather than the behavior driven “London” school of TDD.

  1. As mentioned in the last section, TDD is a development approach. It’s a development approach that happens to produce unit tests as you go, which you can then save for later. I suppose you could discard them and retain some of the benefit of the approach, but that would certainly be a waste since you’re going to want automated tests for your system anyway.
  2. Another facet of the test driven development approach is that you avoid “paralysis by analysis,” a situation in which you are so overwhelmed by the complexity of the problem that you simply stare at the screen or otherwise procrastinate, unsure how to proceed. TDD ensures that you’re constantly solving manageable problems.
  3. TDD produces test cases that cover and address every line of code that you write. With this 100% test coverage, you can change your code fearlessly in response to altered requirements, architectural needs, or other unforeseen circumstances. You’ll never have to look at the system nervously, wondering if you’re breaking things. Your tests will tell you.
  4. Besides allowing you to change code easily, test driven development also guides you toward a flexible design. The reason for this is that TDD forces you to assemble your code with testing seams in it, which are entry points, such as constructor and method arguments, that allow you to take advantage of polymorphism for easier testing. A side effect of this is that your code that’s testable is also easier to configure and mix and match in production.
  5. TDD is also a way to ensure that you’re writing as little code as necessary. Since every change to production code requires a failing test, you have to think through exactly what the system needs before you ever touch it. This prevents speculative coding and throwing in things that you assume you’ll need later, such as property setters you never actually use.

So how is all of this accomplished? Well, TDD is a discipline that follows a specific process and relatively simple process.

  1. As I mentioned briefly, the first step is that you expose some kind of deficiency in the system that you’d like to address. This could be a bug, a missing feature, a new requirement – anything that your codebase does not currently do that you want it to. With the deficiency picked out, you write a test that fails because of the deficiency.
  2. With the failing test in place, you then implement the simplest possible solution in your code to get the failing test to pass, ensuring that all of your other tests also still pass. This makes the system completely functional.
  3. Once the system is completely functional, you look at your quick and dirty fix and see if it needs to be refactored toward better design. If so, you perform the refactoring, ensuring that the tests still pass.

So there you have it: a brief overview of what TDD really is. If you’re interested in more on this subject and you have a Pluralsight subscription, check out my course on continuous testing TDD using a tool called NCrunch, which is all about speeding up your feedback loop during development. Most of this post is from the transcript of that course. If you don’t have a Pluralsight subscription and are interested in a trial, drop me a line and I’ll give you a free week of Pluralsight subscription.

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.

Feb
11

By

TDD: Simplest is not Stupidest

Category: Language Agnostic Tags: , | 4 Comments

Where the Message Gets Lost In Teaching TDD

I recently answered a programmers’ stackexchange post about test-driven development. (As an aside, it will be cool when me linking to a SE question drives more traffic their way than them linking to me drives my way 🙂 ). As I’m wont to do, I said a lot in the answer there, but I’d like to expand a facet of my answer into a blog post that hopefully clarifies an aspect of Test-Driven Development (TDD) for people–at least, for people who see the practice the way that I do.

One of the mainstays of showcasing test-driven development is to show some extremely bone-headed ways to get tests to pass. I do this myself when I’m showing people how to follow TDD, and the reason is to drive home the point “do the simplest thing.” For instance, I was recently putting together such a demo and started out with the following code:

[TestMethod, Owner("ebd"), TestCategory("Proven"), TestCategory("Unit")]
public void IsEven_Returns_False_For_1()
{
    var inspector = new NumberInspector();
 
    Assert.IsFalse(inspector.IsEven(1));
}

public class NumberInspector
{
    public bool IsEven(int target)
    {
        return false;
    }
}

This is how the code looked after going from the “red” to the “green” portion of the cycle. When I used CodeRush to define the IsEven method, it defaulted to throwing NotImplementedException, which constituted a failure. To make it pass, I just changed that to “return false.”

The reason that this is such a common way to explain TDD is that the practice is generally being introduced to people who are used to approaching problems monolithically, as described in this post I wrote a while back. For people used to solving problems this way, the question isn’t, “how do I get the right value for one,” but rather, “how do I solve it for all integers and how do I ensure that it runs in constant time and is the modulo operator as efficient as bit shifting and what do I do if the user wants to do it for decimal types should I truncate or round or throw an exception and whoah, I’m freaking out man!” There’s a tendency, often fired in the forge of undergrad CS programs, to believe that the entire algorithm has to be conceived, envisioned, and drawn up in its entirety before the first character of code is written.

So TDD is taught the way it is to provide contrast. I show people an example like this to say, “forget all that other stuff–all you need to do is get this one test passing for this one input and just assume that this will be the only input ever, go, now!” TDD is supposed to be fast, and it’s supposed to help you solve just one problem at a time. The fact that returning false won’t work for two isn’t your problem–it’s the problem of you forty-five seconds from now, so there’s no reason for you to bother with it. Live a little–procrastinate!

You refine your algorithm only as the inputs mandate it, and you pick your inputs so as to get the code doing what you want. For instance, after putting in the “return false” and getting the first test passing, it’s pretty apparent that this won’t work for the input “2”. So now you’ve got your next problem–you write the test for 2 and then you set about getting it to pass, say with “return target == 2”. That’s still not great. But it’s better, it was fast, and now your code solves two cases instead of just the one.

Running off the Rails

But there is a tendency I think, as demonstrated by Kristof’s question, for TDD teachers to give the wrong impression. If you were to write a test for 3, “return target == 2” would pass and you might move on to 4. What do you do at 4? How about “return target == 2 || target == 4;”

So far we’ve been moving in a good direction, but if you take off your “simplest thing” hat for a moment and think about basic programming and math, you can see that we’re starting down a pretty ominous path. After throwing in a 6 and an 8 to the or clause, you might simply decide to use a loop to iterate through all even numbers up to int.MaxValue, or-ing a return value with itself to see if target is any of them.

public bool IsEven(int target)
{
    bool isEven = false;
    for (int index = 0; index < int.MaxValue - 1; index += 2)
        isEven |= target == index;
    return isEven;
}

Yikes! What went wrong? How did we wind up doing something so obtuse following the red-green-refactor principles of TDD? Two considerations, one reason: "simplest" isn't "stupidest."

Simplicity Reconsidered

The first consideration is that simple-complex is not measured on the same scale as stupid-clever. The two have a case-by-case, often interconnected relationship, but simple and stupid aren't the same just as complex and clever aren't the same. So the fact that something is the first thing you think of or the most brainless thing that you think of doesn't mean that it's the simplest thing you could think of. What's the simplest way to get an empty boolean method to return false? "return false;" has no branches and one hardcoded piece of logic. What's the simplest way that you could get a boolean method to return false for 1 and true for 2? "return target == 2" accomplishes the task with a single conditional of incredibly simple math. How about false for 1 and true for 2 and 4? "return target % 2 == 0" accomplishes the task with a single conditional of slightly more involved math. "return target == 2 || target == 4" accomplishes the task with a single conditional containing two clauses (could also be two conditionals). Modulo arithmetic is more elegant/sophisticated, but it is also simpler.

Now, I fully understand the importance in TDD of proceeding methodically and solving problems in cadence. If you can't think of the modulo solution, it's perfectly valid to use the or condition and put in another data point such as testing for IsEven(6). Or perhaps you get all tests passing with the more obtuse solution and then spend the refactor phase refining the algorithm. Certainly nothing wrong with either approach, but at some point you have to make the jump from obtuse to simple, and the real "aha!" moment with TDD comes when you start to recognize the fundamental difference between the two, which is what I'll call the second consideration.

The second consideration is that "simplest" advances an algorithm where "stupidest" does not. To understand what I mean, consider this table:

ConditionalClauseChart

In every case that you add a test, you're adding complexity to the method. This is ultimately not sustainable. You'll never wind up sticking code in production if you need to modify the algorithm every time a new input is sent your way. Well, I shouldn't say never--the Brute Forces are busily cranking these out for you to enjoy on the Daily WTF. But you aren't Brute Force--TDD isn't his style. And because you're not, you need to use either the green or refactor phase to do the simplest possible thing to advance your algorithm.

A great way to do this is to take stock after each cycle, before you write your next failing test and clarify to yourself how you've gotten closer to being done. After the green-refactor, you should be able to note a game-changing refinement. For instance:

MilestoneTddChart

Notice the difference here. In the first two entries, we make real progress. We go from no method to a method and then from a method with one hard-coded value to one that can make the distinction we want for a limited set of values. On the next line, our gains are purely superficial--we grow our limited set from distinguishing between 2 values to 3. That's not good enough, so we can use the refactor cycle to go from our limited set to the complete set.

It might not always be possible to go from limited to complete like that, but you should get somewhere. Maybe you somehow handle all values under 100 or all positive or all negative values. Whatever the case may be, it should cover more ground and be more general. Because really, TDD at its core is a mechanism to help you start with concrete test cases and tease out an algorithm that becomes increasingly generalized.

So please remember that the discipline isn't to do the stupidest or most obtuse thing that works. The discipline is to break a large problem into a series of comparably simple problems that you can solve in sequence without getting ahead of yourself. And this is achieved by simplicity and generalizing--not by brute force.

Jan
6

By

Betrayed by Your Test Runner

Category: Other Languages Tags: , , , |

The Halcyon Days of Yore

I was writing some code for Apex this morning and I had a strange sense of deja vu. Without going into painful details, Salesforce is a cloud-based CRM solution and Apex is its proprietary programming language that developers can use to customize their applications. The language is sort of reminiscent of a stripped-down hybrid of Java and C#. The IDE you use to develop this code is Eclipse, armed with an Apex plugin.

The deja vu that I experienced transported me back to my college days working in a 200 oldschoollinuxlevel computer systems course where projects assigned to us were the kind of deal involving profs/TAs writing 95% of the code and we filled in the other 5%. I am always grateful to my alma mater for this since one of the things most lacking in university CS education is often concepts like integration and working on large systems. In this particular class, I was writing C code in pico and using a makefile to handle compiling and linking the code on a remote server. This generally took a while because of network latency, server business, it being 13 years ago, a lot of files to link, etc. The end result was that I would generally write a lot of code, run make, and then get up and stretch my legs or get a drink or something, returning later to see what had happened.

This is what developing in Apex reminds me of. But there’s an interesting characteristic of Apex, which is that you have to write unit tests, they have to pass, and they have to cover something like 70% of your code before you’re allowed to run it on their hardware in production. How awesome is that? Don’t you sometimes wish C# or Java enforced that on your coworkers that steal in like ninjas and break half the things in your code base with their checkins? I was pumped when I got this assignment and set about doing TDD, which I’ve done the whole time. I don’t actually know what the minimum coverage is because I’ve been at 100% the entire time.

A Mixed Blessing?

One of the first things that I thought while spacing out and waiting for compile was how much it reminded me of my undergrad days. The second thing I thought of, ruefully, was how much better served I would have been back then to know about unit tests or TDD. I bet that could have same me some maddening debugging sessions. But then again, would I have been better off doing TDD then? And, more interestingly, am I better off doing it now?

Anyone who follows this blog will probably think I’ve flipped my lid and done a sudden 180 on the subject, but that’s not really the case. Consider the following properties of Apex development:

  1. Sometimes when you save, the IDE hangs because files have to go back to the server.
  2. Depending on server load, compile may take a fraction of a second or up to a minute.
  3. It is possible for the source you’re looking at to get out of sync with the feedback from the compiling/testing.
  4. Tests in a class often take minutes to run.
  5. Your whole test suite often takes many, many minutes to run.
  6. Presumably due to server load balancing, network latency and other such factors, feedback time appears entirely non-deterministic.
  7. It’s normal for me to need to close Eclipse via task manager and try again later.

Effective TDD has a goal of producing clean code that clearly meets requirements at the unit level, but it demands certain things of the developer and the development environment.  It is not effective when the feedback loop is extremely slow (or worse, inaccurate) since TDD, by its nature, requires near constant execution of unit tests and for those unit tests to be dependable.

Absent that basic requirement, the TDD practitioner is faced with a conundrum.  Do you stick to the practice where you have red (wait 2 minutes), green (what was I doing again, oh yeah, wait 3 minutes), refactor (oops, I was reading reddit and forgot what I was doing)?  Or do you give yourself larger chunks of time without feedback so that you aren’t interrupted and thrown out of the flow as often?

My advice would be to add “none of the above” to the survey and figure out how to make the feedback loop tighter.  Perhaps, in this case, one might investigate a way to compile/test offline, alter the design, or to optimize somehow.  Perhaps one might even consider a different technology.  I’d rather switch techs than switch away from TDD, myself.  But in the end, if none of these things proves tenable, you might be stuck taking an approach more like one from 20+ years ago: spend a big chunk of time writing code, run it, write down everything that went wrong, and trying again.  I’ll call this RDD — restriction driven development.  I’d say it’s to be avoided at all costs.

I give force.com an A for effort and concept in demanding quality from developers, but I’d definitely have to dock them for the implementation since they create a feedback loop that actively discourages the same.  I’ve got my fingers crossed that as they expand and improve the platform, this will be fixed.

Nov
12

By

TDD For Breaking Problems Apart 3: Finishing Up

Category: .NET Tags: , , , | 5 Comments

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

Nov
6

By

TDD For Breaking Problems Apart 2: Magic Boxes

Category: .NET Tags: , , , | 1 Comment

In the last post, I started a TDD exploration of the “bowling score” code kata. Today, I’m going to build a “magic box” in the course of TDD to solve the problem that arose at the end of the post with the scoring class doing too much.

Last time, we left off with production code that looked like this:

public class BowlingScoreCalculator
{
    public const int Mark = 10;

    public class FrameUnderflowException : Exception { }

    public class FrameOverflowException : Exception { }

    public int Score { get; private set; }

    public void BowlFrame(int firstThrow, int secondThrow)
    {
        if (firstThrow < 0 || secondThrow < 0)
            throw new FrameUnderflowException();
        else if (firstThrow + secondThrow > Mark)
            throw new FrameOverflowException();

        Score += firstThrow + secondThrow;
    }
}

And the last test we had written looked like this:

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

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

I had noticed that the amount of integer literals here seemed to be a bit of a smell and was starting to wrap my head around the idea of what to do about it from a design perspective. Specifically, the smell in question is “primitive obsession,” a tendency to overuse primitive types, often redundantly, due to reluctance to make an object. So what if we made an object — a frame object — what would that look like?

Looking at the BowlFrame() method, I think “what if I had a magic box that handled frame operations?” Well, the method would take a frame object and presumably that object would handle validating the individual throws, so the method would lose that exception handling logic (and the class would probably lose those exception definitions). It would probably also make sense for the frame to encapsulate some kind of totaling mechanism internally so that the score calculator didn’t need to inspect its properties to figure out what to add.

At this point, though, I’m going to stop and start coding up the frame class as I envision it. After the first test (including a couple of non-compile failures and passes prior to the test run failure), this is what the frame test and production code looks like:

[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);
        }
    }
}

public class Frame
{
    public int FirstThrow { get; private set; }

    public Frame(int firstThrow, int secondThrow)
    {
        FirstThrow = firstThrow;
    }
}

The first thing I do at this point is refactor the test to eliminate the magic numbers and duplication. At this point, you might wonder why, having done this enough times, I don’t simply start out by leaving out the magic numbers and duplication. Am I obtuse? Well, sometimes, but the reason I often don’t bother with this is that I want the test I’m writing to be the simplest possible thing as I perceive it for problem solving purposes — not necessarily the quickest and certainly not the cleanest. I wouldn’t ever purposefully make a mess, but if calling the frame constructor with (1, 0) seems more intuitive to me, then that’s what I do. I encourage you to do the same. Simplicity as you perceive it is of the utmost importance writing the failing test. Whatever it is, make it pretty and elegant later.

The next thing I do is add a similar test and then code for the second throw, with similar green and refactor. With that in place, I move the exception handling tests from the ScoreCalculator test class to the Frame test class and change them to operate on Frame’s constructor. When I do this, I get a red test, which I make green by porting the exception handling code and constants over to the new Frame class. The production code is now:

public class BowlingScoreCalculator
{
    public int Score { get; private set; }

    public void BowlFrame(int firstThrow, int secondThrow)
    {
        Score += firstThrow + secondThrow;
    }
}

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 Frame(int firstThrow, int secondThrow)
    {
        if (firstThrow < 0 || secondThrow < 0)
            throw new UnderflowException();
        if (firstThrow + secondThrow > Mark)
            throw new OverflowException();

        FirstThrow = firstThrow;
        SecondThrow = secondThrow;
    }
}

I’m happy that all of that logic about determining what constitutes a valid frame is now out of the BowlingScoreCalculator class, but it’s not yet sitting inside of a magic box — it’s just gone. So, let’s give the calculator the box that it’s looking for. We’re going to need a failing test, and the easiest way to do that is to change a test where we’re bowling a frame to look like this:

[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);
}

That doesn’t compile, so we can fix the broken test by adding this method in production:

public class BowlingScoreCalculator
{
    public int Score { get; private set; }

    public void BowlFrame(int firstThrow, int secondThrow)
    {
        Score += firstThrow + secondThrow;
    }

    public void BowlFrame(Frame frame)
    {
        BowlFrame(frame.FirstThrow, frame.SecondThrow);
    }
}

At this point, we can refactor the rest of the tests to use the second method and then refactor that method to inline the original method and now the score calculator only accepts frames. There’s our magic box. Suddenly, we’re just worrying about how to add the scores from the frame’s throws and not whether the frame we’re being passed is valid or not. And that makes sense — we shouldn’t care about the anatomy of a frame in a class responsible for adding up and tracking frames. We should be able to assume that if the frame exists and is being handed to us that it’s valid.

Now that this bigger refactoring is done, there’s still something a little fishy. Look at this test and production code for the calculator:

[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.FirstThrow + frame.SecondThrow + frame.FirstThrow + frame.SecondThrow, Target.Score);
}

public class BowlingScoreCalculator
{
    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        Score += frame.FirstThrow + frame.SecondThrow;
    }
}

We’ve fixed our primitive obsession, but there sure seems to be a lot of redundancy. I mean I have to ask frame for two things in the production code and four things in the test: two for each frame. What if I had a magic box that turned those two things into one? What would that look like? Well, here’s a test I can write against frame that I think will tell me:

[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);
}

Now I just need to define that property as the sum of my two throws to make it pass. I do that, and now I can refactor my score calculator and test. Here’s what it looks like now:

[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);
}

public class BowlingScoreCalculator
{
    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        Score += frame.Total;
    }
}

Now, all of the frame stuff is removed from score calculation and those concerns are separated. The “magic box,” Frame, handles evaluating frames for validity and totaling them up. This score calculator class is actually starting to smell like a lazy class that could be replaced with a list and a call to a Linq extension method, but I’m going to keep it around on a hunch.

Okay, now that we’ve done a good bit of cleanup and pulled out a magic box, time to get back to implementing features. Off the top of my head, I can think of two scenarios that we don’t currently handle: the 10th frame and marks carrying over from previous frames. I’m going to work on the latter for now and write a test that when I bowl a spare in the first frame and a 5, 0 in the second frame, my score should be 20 instead of 15.

[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);
}

This test fails and I need to get it to pass. There are no trivial tricks I can do, but I will do the simplest thing I can think of. I’ll store an array, write frames to it, and check last frame to know when to handle this case. Here is the simplest thing I can think of that makes this pass:

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

    private int _currentFrame;

    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        if (_currentFrame > 0 && _frames[_currentFrame - 1].Total == Frame.Mark)
            Score += frame.FirstThrow;

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

That’s pretty ugly though, so let’s clean it up at least a little before we move on. Boy Scout Rule and all that:

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

    private int _currentFrame;

    public int Score { get; private set; }

    public void BowlFrame(Frame frame)
    {
        if (WasLastFrameAMark())
            Score += frame.FirstThrow;

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

    private bool WasLastFrameAMark()
    {
        return _currentFrame > 0 && _frames[_currentFrame - 1].Total == Frame.Mark;
    }
}

Now the method is semantically clearer than it was with all of that crap in the guard condition. If the last frame was a mark, do something different with this frame, and then do our normal score augmentation and frame recording. We might decide to do some reasoning at this point about whether score should be computed as frames are added or only on demand, but that’s a little nitpicky, so let’s instead focus on the fact that this doesn’t score strikes correctly. So we’ll write a test in which we bowl a strike in the first frame and then a (6, 4) in the second frame and assert that the score should be 30 (unlike the current implementation, which would make it 26):

[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);
}

And, let’s fix the code:

public void BowlFrame(Frame frame)
{
    if (_currentFrame > 0 && _frames[_currentFrame - 1].FirstThrow == Frame.Mark)
        Score += frame.Total;
    else if (WasLastFrameAMark())
        Score += frame.FirstThrow;

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

That makes everyone green, but it’s ugly, so let’s refactor it to this:

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

    private int _currentFrame;

    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 && _frames[_currentFrame - 1].FirstThrow == Frame.Mark;
    }
    private bool WasLastFrameASpare()
    {
        return _currentFrame > 0 && _frames[_currentFrame - 1].Total == Frame.Mark;
    }
}

Now BowlFrame() is starting to read like the actual scoring rules of bowling. “If the last frame was a strike, add current frame’s total points to the score as a bonus. Otherwise, if it was a spare, add the first throw’s points to the score as a bonus. No matter what, add this frame’s pins.” Not too shabby. At this point, before proceeding any further with the score calculator (such as handling double strikes and the 10th frame), I’d like to clean up something I don’t like: the fact that the score class is responsible for figuring out whether a frame is a strike or spare. I’m eliding the details of the tests I wrote for the Frame class to make this happen, but they’re pretty straightforward and I’ll post everything at the end of the series. Here is the new look calculator, after some refactoring:

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;
    }
}

Next time, I’ll cover the addition of double strike logic and the tenth frame, and I’ll explore some different alternatives that are easily achievable with a nice suite of unit tests backing things up.

« Older Posts
Newer Posts »