I’ve been thinking about the second roll and I think I have a plan. Also: Resist fascism, and take care of one another!

The rough plan is this: the first box (strike?) will only consume a first roll of ten. If it gets any other roll, it will accept it and go to satisfied, but will not consume the roll. That will allow the first roll to show up at the second (spare?) box, where it will be recorded and consumed. The spare box’s state machine will be similar to the strike box, either becoming satisfied if the second roll plus the first do not equal 10, or accepting a third but roll but not consuming it, allowing that bonus roll to roll on down stream.

I’ll draw that diagram in a moment. First, let’s get at least one test in place.

(I would normally only write one test at a time, but I may want to lay out alternatives before I start. Perhaps I should just write the proposed tests on a card. We’ll see.)

It’s past time to rename the Box class as well. I think we’ll call it StrikeBox for now, and use SpareBox for the one we’re about to work on. PyCharm politely offers to rename the file as well. Love those people!

    def test_spare_box_spare(self):
        spare_box = SpareBox()
        spare_box.add_score(6)
        spare_box.add_score(4)
        spare_box.add_score(3)
        assert spare_box.score() == 13

So, a spare. Let’s draw the machine diagram.

spare box state diagram

The code is straightforward, very similar to the StrikeBox:

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

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

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

    def _record_first_roll(self, number):
        self._scores.append(number)
        self._next_action = self._record_second_roll
        return True

    def _record_second_roll(self, number):
        self._scores.append(number)
        self._next_action = self._satisfied if sum(self._scores) < 10 else self._record_bonus
        return True

    def _record_bonus(self, number):
        self._scores.append(number)
        self._next_action = self._satisfied
        return False

    def _satisfied(self, number):
        return False

The test passes. Again I have gone beyond my remit, in that I’m dealing with the non-spare case (I think), and returning the consumed flag. I think we need to do better than True and False for that, by the way.

Green, though. Commit: first cut at SpareBox

    def test_spare_box_open(self):
        spare_box = SpareBox()
        spare_box.add_score(6)
        spare_box.add_score(3)
        spare_box.add_score(4)  # should be ignored
        assert spare_box.score() == 9

That passes. Let’s check the returns. We’ll use new tests: I’m trying to learn to write simpler tests.

    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

That passes as well. We haven’t tested for incomplete data returning None. There are two cases:

    def test_spare_box_not_ready(self):
        spare_box = SpareBox()
        assert spare_box.score() is None
        spare_box.add_score(5)
        assert spare_box.score() is None
        spare_box.add_score(5)
        assert spare_box.score() is None
        spare_box.add_score(5)
        assert spare_box.score() is 15

Passes. Commit: SpareBox passes all tests. Let’s reflect: how are we doing, and what’s next?

Reflection

There is what you might call a lot of duplication between StrikeBox and SpareBox. I think I could rename the methods and make almost everything a complete duplicate. But there are critical differences as well: the StrikeBox makes its big decision on the first roll: am I a strike or not? and the SpareBox makes its decision on the second roll. We could certainly combine the two classes and change only the init to set a first state, and then reuse some of the operations, but that seems likely to make it harder to understand.

The public methods, add_score and score are identical. We could remove that by inheritance, which would earn me a look askance from some folx, or by use of a strategy object containing just the machine part. We might look into that later. For now, keeping them separate seems best to me, because we still have the marks to deal with and we haven’t even plugged these objects into a larger scheme, either a frame notion, if we have one, or the whole game. And there’s still the tenth frame boxes to consider.

I like the state machine implementation and if you hate it, please toot me up and we’ll kick it around. State machines are perhaps a bit deep in the bag of tricks for some teams, though not here chez Ron. Still, it’s fair to say that they are sometimes harder to follow than conditional statements. The main point of this implementation is to explore Yet Another Way of doing bowling scoring, and it may or may not be a Good Way.

I think it’s time to test the two boxes working in concert. In particular, I expect that the StrikeBox needs at least one change, so that the second box will see the first roll. By the way … the StrikeBox wants to keep the first roll even if it is not a strike, because it’s job will be to display either an X if the frame is a strike frame, or the first roll value if not.

So, a test. We’ll pretend to be the scorekeeper object that will hold the Boxes and pass rolls to them.

I wonder if we should split the test class into one file testing strike boxes and a separate file for spare boxes. Let’s try it. I rename the first test class, insert a second test class ahead of the spare tests, move to another file, rename a file. All tests still running. Commit: split test files.

Now a new test file, test_scoring, I think we’ll call it.

class TestScoring:
    def roll(self, roll, b1, b2):
        consumed = b1.add_score(roll)
        if not consumed:
            b2.add_score(roll)

    def test_two_boxes_open(self):
        strike_box = StrikeBox()
        spare_box = SpareBox()
        self.roll(5, strike_box, spare_box)
        self.roll(4, strike_box, spare_box)
        assert spare_box.score() is 9

I am surprised to find this not passing. It turns out that StrikeBox is reporting consumed on the first ball. I though I had changed that but I guess I didn’t. Fine, we have a failing test, we can fix it.

class StrikeBox:
    def _record_first_roll(self, number):
        self._scores.append(number)
        self._next_action = self._record_first_bonus \
            if number == 10 \
            else self._satisfied
        return True

Sure enough, we always return True. We want to fix that and let’s write a direct tests for it as well.

    def _record_first_roll(self, number):
        self._scores.append(number)
        if number == 10:
            self._next_action = self._record_first_bonus
            return True
        else:
            self._next_action = self._satisfied
            return False

That gets me green. Commit: first scoring test passing.

Now some direct testing for the returns from StrikeBox. We have this test already:

    def test_strike_box_consumes_just_one(self):
        box = StrikeBox()
        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

Rename that one, add a new one:

    def test_strike_box_consumes_one_on_strike(self):
        box = StrikeBox()
        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

    def test_strike_box_consumes_none_on_non_strike(self):
        box = StrikeBox()
        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

Commit: direct tests for returns from add_score

There is one more check needed in the first scoring test:

    def test_two_boxes_open(self):
        strike_box = StrikeBox()
        spare_box = SpareBox()
        self.roll(5, strike_box, spare_box)
        self.roll(4, strike_box, spare_box)
        assert spare_box.score() is 9
        assert strike_box.score() is None

This is a design change: If the strike box is not recording a strike, I want it to return None as its score, because I’m planning to sum the scores from the two boxes to get the frame score, so I want one to return the score and the other to return None. (Zero might be OK: I’m not sure about that.)

In fact, I’m not sure about any of this. Having just worked with it, I’m realizing that since the strike box will consume the first roll on a strike, the spare box will see the second roll, but if the first roll is not a strike, spare box will see both first and second roll.

Possibly the strike frame never consumes anything?

Interesting possibility. I think the tests will tell us. I’m a bit concerned that the idea may not pan out.

But wait!

If the separate boxes is a bad idea, why is that a reason to be concerned? We’re exploring an interesting possibility for scoring. If it doesn’t work, that’s just as interesting as if it does. It’s not like I laid down a big bet on this idea.

The StrikeBox will return a score only if it has read three rolls. Let’s change it to pass the test. I think I’ll give it two end states.

    def _record_first_roll(self, number):
        self._scores.append(number)
        if number == 10:
            self._next_action = self._record_first_bonus
            return True
        else:
            self._next_action = self._inactive
            return False

    def _inactive(self, _number):
        return False

Two tests are now failing. I hope they are old ones.

    def test_strike_box_open_unsatisfied(self):
        box = StrikeBox()
        assert box.score() is None
        box.add_score(5)
        assert box.score() is 5

Yes, new rule, it should be None. And:

    def test_strike_box_non_strike(self):
        box = StrikeBox()
        box.add_score(8)
        box.add_score(1)
        box.add_score(5)
        assert len(box._scores) == 1
        assert box.score() == 8

Right, we want that to be None as well. StrikeBox never has a score for a non strike! With those changes, we’re green and our first scoring test is passing. Commit: first scoring test OK

    def test_two_boxes_spare(self):
        strike_box = StrikeBox()
        spare_box = SpareBox()
        self.roll(6, strike_box, spare_box)
        self.roll(4, strike_box, spare_box)
        self.roll(3, strike_box, spare_box)
        assert spare_box.score() is 13
        assert strike_box.score() is None

Green. Commit: spare scoring OK

One more:

    def test_two_boxes_strike(self):
        strike_box = StrikeBox()
        spare_box = SpareBox()
        self.roll(10, strike_box, spare_box)
        self.roll(10, strike_box, spare_box)
        self.roll(9, strike_box, spare_box)
        assert spare_box.score() is None
        assert strike_box.score() is 29

Commit: strike scoring OK

Time to come up for air.

Reflection

We’re an hour and 40 minutes in. Might be a good stopping point.

I am pleased in several regards:

  1. The StrikeBox and SpareBox are working individually and together, and there is no actual connection between them.

  2. The tests are getting quite small, although they do require more than one roll to get a decent test. That’s not surprising, since we really only care about the end states and results. And I’m proud of having three separate test files with only 14 tests. That’s probably a personal best.

  3. Changes were needed in StrikeBox to get it cooperating with SpareBox, but those changes went easily. I had only recognized that it might be possible to get the boxes to cooperate without a connection early this morning, so that’s quite satisfactory.

  4. I figured out how to get a photo from my phone to synchronize with the Mac when I need it to.

What is there to be concerned about?

  1. The big open question, I think, is how things will work out when there is a whole row of StrikeBox / SpareBox /StrikeBox / … to deal with. I think it’ll work just fine, but we’ll find out when we test it.

  2. I am tempted to jump to at least nine frames. I would probably be wise to take smaller steps. How can I know that? Because I can’t think of a time when I took a step and then thought Wow! That was too small.

  3. The tenth frame has three boxes, unlike all other frames with two. Will our existing ones be useful here or will we need two or three new boxes? No way to know until we get to working on it. We certainly could work on the tenth frame next. Might be wise.

  4. Bowling does have the notion of a frame and we need to get the cumulative score by frame, not by “some little box somewhere”. I can think of at least one way to deal with that: a collection of pairs (and one trio) of boxes. Might not even need an object. Probably should have one, though, for such a key bowling concept.

  5. There is duplication in the boxes, including their __init__, add_score, and score methods, with some others also seeming quite similar. Ideally, we could exploit that duplication without making things less clear.

  6. It isn’t very communicative to return True and False to mean something like “pass this ball on down”. I think we’d do well to do something about that. Even just some named constants in the state machines might help.

Summary

Overall, I think it’s going smoothly, and I look forward to comparing this solution to others. I don’t expect it to win, necessarily. The mission is to try something, practicing small steps driven by small tests, and to see how much we like it, be that a lot or a little.

I said “driven by tests” just there. It’s worth noting that when entering those state machines, I put in more code than the tests currently called for, essentially every time. It would be possible to test one state at a time, I suppose, but that would make for a very invasive, and therefore very brittle test. Since I had the state diagrams and they were fresh in my mind, and since PyCharm complains if all the methods you reference aren’t defined, it is perfectly OK with me to have worked that way: the steps were still quite small, and we were quite careful to add the tests that covered the behavior that was added “prematurely”.

Personal Responsibility

You can find experts who will tell you, in bombastic speeches or heavy books, that you’re not a good developer unless you follow these specific and highly acronymized draconian rules. I am not that expert. I am here to show you what I do, which is always imperfect, and what happens, which is that I sometimes get in trouble and then get out of it, or often create code that isn’t very good and refactor it until it is rather good. I tell you what really happens, what I’m really thinking and feeling, and leave it to you to decide your best way of working.

Of course I have an opinion that sounds a lot like “I think you’d do better to do …”, but I don’t know how you work, I don’t know what you experience, and I certainly don’t know what will make your life better. Obviously I try to make my best choices, and they are quite consistent. And, equally obviously, I fumble, make mistakes, do the wrong thing. I’m just zis guy, you know?

So you do you, and I hope you enjoy watching me do me. See you next time!

Resist fascism, and take care of one another! (Not a command, a heartfelt request.)