One Machine ...

… to rule them all. I thought this was too hard Now I think it’s just right. Which is it? [Man bites bear!] Also: Resist fascism, and take care of one another!

I like the simple state machines that we have so far, the StrikeBox and SpareBox. A few days back, when I was first thinking about using state machines for the bowling scoring, I tried to draw a diagram of the whole scoring process and decided that it was too difficult. Yesterday, while avoiding some program that my wife was watching, I started up Procreate and drew this diagram:

Since it was going to be just between you and me, I felt no need to fancy it up with some high-tech program. That sketch was enough to make me believe that we can probably do the whole job with just one machine, much simpler than I had thought it would be.

What made the difference?

Well, I think two things. One was that now I have actually built two of these little machines, and so they seem generally easier to do. The other is that, if I recall, I started out with a rather complex diagram, and was trying to make it reenter used nodes from other places, and it started to look like a spider web built by a spider on caffeine. (Look it up, you won’t be sorry.)

So, we’re going to try to build the One Machine to Rule Them All. I figure that it’ll be easy enough to see if it’s going to work. We have tests that we can reuse. It will become clear quite quickly be harder to handle, so we’ll want to get to that as soon as we can, unless experience on the way moves us in another direction.

I can think of at least two ways to go:¹

1. Use existing tests and edit one of the existing machines to deal with the other type’s tests.
2. Write a new machine and test it by editing the existing tests.
3. Write a whole new tranche² of tests and a new machine class.

The tension that I feel between these is that the first two feel like less work, but they make it hard to commit along the way, since we might decide this is a bad idea and want to roll back to this morning. I hate rolling back more than one commit’s worth.

Let’s do #3 and see if we can make it easy. It’ll certainly be OK to borrow / copy tests from the other test files.

New test file.

class TestRuler:
    def test_hook(self):
        assert True

So far so good. But do I really want to copy over all 14 tests? I do not. A little searching tells me that pytest can parameterize a test. Let’s try it in one of them.

class TestStrikeBoxes:
    @pytest.mark.parametrize("cls", [StrikeBox, StrikeBox])
    def test_strike_box_consumes_one_on_strike(self, cls):
        box = cls()
        first_consumed = box.add_score(10)
        assert first_consumed == True
        second_consumed = box.add_score(5)
        assert second_consumed == False
        third_consumed = box.add_score(4)
        assert third_consumed == False

Sure enough, that runs (and passes) one more test, the second StrikeBox instance.

OK, this will be slightly tedious but I can do it. I’ll make a variable to contain the list.

classes = [StrikeBox]
class TestStrikeBoxes:
    @pytest.mark.parametrize("cls", classes)
    ...

Once I’ve decorated all the tests in the file, I can add my new class to the list and bingo they’re all running it.

Or, more conservatively, I can parameterize them one at a time, at least at first.

Let’s do that … it will leave us in a committable³ state more often. We need a name for the new box. I’ll just call it Box for now, because I Just Don’t Know.

    @pytest.mark.parametrize("cls", classes)
    def test_strike_box_consumes_one_on_strike(self, cls):
        box = cls()
>       first_consumed = box.add_score(10)
E       AttributeError: 'Box' object has no attribute 'add_score'

We’re on our way. I am so tempted—so tempted—to copy-paste but let’s work incrementally.

class Box:
    def add_score(self):
        pass

>       first_consumed = box.add_score(10)
E       TypeError: Box.add_score() takes 1 positional argument but 2 were given

Yes, I am going this stupidly. Perhaps I’ll dial it back a bit.

class Box:
    def __init__(self):
        self._scores = []
        self._next_action = self._record_first_roll

    def add_score(self, pins):
        return self._next_action(pins)

I’ll chunk forward a bit and show you want I get. I’ll follow the state diagram and at least look at the other objects. It’s not a contest to see how ignorant I can be. I just don’t want to copy something directly unless I’m really OK with it.

class Box:
    def __init__(self):
        self._scores = []
        self._next_action = self._record_first_roll

    def add_score(self, pins):
        return self._next_action(pins)

    def _record_first_roll(self, pins):
        self._scores.append(pins)
        self._next_action = self._strike
        return True

    def _strike(self, pins):
        self._scores.append(pins)
        self._next_action = self._stike_bonus
        return False

    def _stike_bonus(self, pins):
        self._scores.append(pins)
        self._next_action = self._was_strike
        return False

    def _was_strike(self, pins):
        return False

The first test passes. Neat. Commit: Box passes first test. We parameterize another test:

    @pytest.mark.parametrize("cls", classes)
    def test_strike_box_consumes_none_on_non_strike(self, cls):
        box = cls()
        first_consumed = box.add_score(9)
        assert first_consumed == False
        second_consumed = box.add_score(5)
        assert second_consumed == False
        third_consumed = box.add_score(4)
        assert third_consumed == False

Ah. no good on this one: our new box will consume the two rolls of a spare or open frame. as we see in this test:

    def test_spare_box_returns(self):
        spare_box = SpareBox()
        assert spare_box.add_score(5) is True
        assert spare_box.add_score(5) is True
        assert spare_box.add_score(5) is False 

Since we are combining the function of the two boxes … well, you get the idea. Let’s unparameterize⁴ the strike test above and parameterize the spare one.

    @pytest.mark.parametrize("cls", classes)
    def test_spare_box_returns(self, cls):
        spare_box = cls()
        assert spare_box.add_score(5) is True
        assert spare_box.add_score(5) is True
        assert spare_box.add_score(5) is False

It fails, of course. Let’s do a bit more than this test calls for.

    def _record_first_roll(self, pins):
        self._scores.append(pins)
        if pins == 10:
            self._next_action = self._strike
        else:
            self._next_action = self._record_second_roll
        return True

    def _record_second_roll(self, pins):
        self._scores.append(pins)
        self._next_action = self._open_frame
        return True

    def _open_frame(self, _pins):
        return False

That wasn’t too much too much, just enough too much. Green. Commit: two-roll frames return correct True/False

Let’s go back to strikes and try to finish up one side of the object. We’ll try this test:

    @pytest.mark.parametrize("cls", classes)
    def test_strike_box_unsatisfied(self, cls):
        box = cls()
        assert box.score() is None
        box.add_score(10)
        assert box.score() is None
        box.add_score(5)
        assert box.score() is None
        box.add_score(4)
        assert box.score() == 19

>       assert box.score() is None
E       AttributeError: 'Box' object has no attribute 'score'. 
        Did you mean: '_scores'?

No, I did not. Thanks for trying, but in fact I meant score:

    def score(self):
        if self._next_action == self._was_strike:
            return sum(self._scores)
        else:
            return None

Along the way I learned that you have to compare methods with == not is. Anyway the test passes, let’s do another. Tell you what, I’ll spare you the details unless something interesting comes up. No sense i you just watching me do rote things. I’ll report commits, starting with this one:

• strike box unsatisfied.
• def test_strike_box_open_unsatisfied(self, cls): (no change needed (NC))
• def test_strike_box_score(self, cls): (NC)
• def test_strike_box_ignores_fourth_score(self, cls): (NC)

Those are all the strike tests that should pass. No changes were needed. I’ll continue with the spare tests:

To pass the first one of those I added some code to the Box:

    def score(self):
        na = self._next_action
        if na == self._was_strike or na == self._was_spare:
            return sum(self._scores)
        else:
            return None

    def _record_second_roll(self, pins):
        self._scores.append(pins)
        if sum(self._scores) == 10:
            self._next_action = self._spare_bonus
        else:
            self._next_action = self._open_frame
        return True

    def _spare_bonus(self, pins):
        self._scores.append(pins)
        self._next_action = self._was_spare
        return False

    def _was_spare(self, pins):
        return False

This is pretty much right off the diagram. We have some small duplication, but I think we probably don’t want to reduce it. Remind me not to do so until we have tested the marks.

More commit:

• def test_spare_box_spare(self, cls):
• def test_spare_box_open(self, cls):
• def test_spare_box_not_ready(self, cls): (NC)

Those went so quickly that I forgot to commit some of them. All the applicable tests from test_strikes and test_spares are running, One more test file, test_scoring.

Those don’t need to work, since they test the interaction between StrikeBox and SpareBox, and that’s no longer an issue.

Since I have the test_ruler file, I’ll copy and edit those tests over there. They’re pretty redundant but better to make them work than answer questions about why we didn’t:

class TestRuler:
    def roll(self, roll, b1, b2):
        b1.add_score(roll)

    def test_ruler_open(self):
        box = Box()
        self.roll(5, box, box)
        self.roll(4, box, box)
        assert box.score() is 9

    def test_ruler_spare(self):
        box = Box()
        self.roll(6, box, box)
        self.roll(4, box, box)
        self.roll(3, box, box)
        assert box.score() is 13

    def test_ruler_strike(self):
        box = Box()
        self.roll(10, box, box)
        self.roll(10, box, box)
        self.roll(9, box, box)
        assert box.score() is 29

Commit: all rule tests replicate scoring tests and working.

Time to stop, I guess. Where are we?

Reflection

The new class Box has all the capability of StrikeBox and SpareBox. So far so good. It is probably not time to burn those other boxes, though, until we verify getting the marks and check the tenth frame. So, today, we have extended some tests to test two classes, and created a new class, Box, that promises to subsume the other two. Will it keep its promise? We’ll find out, but I am confident that it will, perhaps with some changes for the tenth frame, but even there I don’t think we’ll have to change the machine’s behavior.

We’ll need to continue the return that indicates whether the roll should be passed on down the line, and we’ll have to build a new ScoreKeeper to create just ten boxes instead of 21 of our Strike, Spare, and various TenthFrame instances. Easily done, I expect.

Most Significant?

It’s significant that we have shown⁵ that a single machine can probably handle all our needs, instead of at least two and probably five.

But I think that the really significant thing today, for me, was using @pytest.mark.parameterize to reuse existing tests on a new class that needs to meet the same criteria. It was much less work than writing new tests from scratch, and required less thinking, because the thinking about what to test was already done. The result of discovering that strategy was to make the rote change to parameterize a test, see it fail (or, quite often pass), make any needed changes, green, commit, move on. It went so smoothly that I actually forgot to commit sometimes.

I think this is a generally useful idea when we’re trying to replace some class with a new class. Instead of editing tests or copying them, we can simply parameterize them in a very rote fashion and tick through making our new class meet the same exact criteria as the old one.

Summary

Sometimes you bite the bear. Today was one of those days. A simplifying idea which will, when finished reduce two classes to one and perhaps avoid writing one, two, or three more. (That is speculative: even with the StrikeBox and SpareBox, we might well have found that we could reuse them in the tenth frame.) And the setup will surely be simpler. Finally, I suspect that the end result will be easier to understand through being generally simpler.

Take that, bear! See you next time! Resist fascism, and take care of one another!