Reset!

FGNO discussion and yesterday’s interruption cause me to start over. It’s going well. Also: Resist fascism, and take care of one another!

Wednesday Morning

I was interrupted yesterday by an appointment before I was at a good stopping point. Then, last night at FGNO, we spoke of spikes, of doing things different ways, and of starting over.¹

The common wisdom of a Spike² is that you code something as an experiment to try something and learn something, and then you throw away the spike and start anew on the real thing. Frequent readers will know that I commonly break that rule, instead keeping the code and continuing to move forward with it. I can only wish this were the worst sin I had ever committed.

We spoke of spikes a bit, in the context of my idea from yesterday of the little boxes catching rolls, and then Bryan brought up an idea based on that discussion, saying that in a TDD or similar class, it would be interesting to give the teams some amount of time, say 90 minutes to do something, and then stop them after 15 minutes and make them throw away the code they had written and start over. Perhaps they’d even be required to solve the problem differently from the way they had been working on.

The points of the deletion exercise, of course, include truly learning that it takes much less time to do something again. The bottleneck in programming is the thinking part, not the typing part, but we often just hate to throw away anything we’ve already typed in.³

Anyway …

I have a few reasons to throw away yesterday’s work. First, I wa interrupted in the middle of it for an appointment. Second, I had already made at least one mistake with the five classes which were very speculative, and which I then fudged by making the last four all inherit from the first. What’s that about? And, finally, I don’t think it was going particularly well anyway.

So pardon me a moment while I delete all the code from yesterday. First, I’ll paste the tests here, so we can think about them, but I’m going to remove everything except the test file and its class name:

class TestBoxes:
    def test_score_machine(self):
        sm = ScoreMachine()
        assert len(sm.boxes) == 2*9 + 3
        boxes = sm.boxes
        for index in range(0, 9):
            assert isinstance(boxes[2*index], NormalFirst)
            assert isinstance(boxes[2*index + 1], NormalSecond)
        assert isinstance(boxes[18], TenthFirst)
        assert isinstance(boxes[19], TenthSecond)
        assert isinstance(boxes[20], TenthThird)

    def test_open_rolls(self):
        sm = ScoreMachine()
        for roll in range(20):
            sm.roll(4)
        for index, box in enumerate(sm.boxes):
            if index < 20:
                assert box.is_closed
                assert box.score == 4
            else:
                assert not box.is_closed
                assert box.score is None

I do that and commit: remove everything. Note that everything was already in Git, so in principle we could get it back if we wanted to. So that might give one a bit of comfort.

OK, review those tests. The first is terribly speculative calling for creation of six classes, ScoreMachine and five kinds of boxes. Why? We have no real idea what the boxes are, or what they do?

Then the second test creates a whole game’s worth of boxes and starts checking them. That test is checking that the scores rolled down to the right boxes. I don’t think it makes much sense to start with six classes, rather than perhaps one, and then to test all six of them at once.

Let’s think now about what I’ve learned from yesterday and my thinking between then and now.

The “idea” is that there would be all these little boxes, I was thinking two per frame except the tenth, and that there would be no frame object, just the boxes. Now there are at least two arguments against there being no frames.

First of all, the definition of the game is in terms of frames. That doesn’t make it a certainty that there should be a Frame class, but betting against that object might be a mistake.

Second, while my attention in the boxes idea is on parsing the rolls into boxes that correspond to the small boxes in a score sheet, we do have to be able to get a frame score.

Finally, even if we had no other use for a frame object, we might need something that enables the little boxes to talk with each other. That’s speculative, but my picture of how they’d work includes the first one telling the second one to ignore any balls (because the first one is a strike). So they might need some kind of container to help with that.

That’s about all that comes to mind right now. The question before us is what we should do now that we have a blank sheet of paper.

Maybe it’s not two or three boxes per frame. Maybe it’s one box with two or three lids that can be closed. Yabbut that seems unlikely to be cohesive. Let’s try working on a “strike box”, the first of the two in a normal frame.

Here’s a largish test but I wrote it to tell myself the story:

    def test_strike_box(self):
        box = Box()
        first_consumed = box.score(10)
        assert first_consumed == True
        second_consumed = box.score(5)
        assert second_consumed == False
        third_consumed = box.score(4)
        assert third_consumed == False

The idea here is that in the case of a strike, the box consumes the first roll, and not the next two (but not shown yet, it does remember them).

I think that even this test is too large, but I can make it run, so let’s do that trivially and then to some finer-grained tests.

I let PyCharm create an empty Box class and then immediately move it to a new file.

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

    def score(self, number):
        self._scores.append(number)
        return len(self._scores) == 1

I could have done this with a flag, but I know I want to retain the scores, so why not? The test passes.

I’ll write another test, though I am tempted to make the one we have even worse.

This is still less granular than I would like:

    def test_strike_box_score(self):
        box = Box()
        box.add_score(10)
        box.add_score(5)
        box.add_score(4)
        assert box.score() == 19

Note that I changed the name of the method that adds a score to add_score, allowing me to use score as the method for getting a score. And I code:

    def score(self):
        if len(self._scores) < 3:
            return None
        return sum(self._scores)

I’m starting to like this. Let’s make that method simpler.

    def score(self):
        return sum(self._scores) if len(self._scores) == 3 else 0

I like the ternary, YMMV. It helps me to see how simple things are.

What should happen with the fourth ball? Let’s break it with a new test.

    def test_strike_box_ignores_fourth_score(self):
        box = Box()
        box.add_score(10)
        box.add_score(5)
        box.add_score(4)
        box.add_score(3)
        assert box.score() == 19

That should fail, answering 22, not 19, no, wait, it’ll return zero because len isn’t 3. Anyway:

    def add_score(self, number):
        if len(self._scores) < 3:
            self._scores.append(number)
        return len(self._scores) == 1

Green. Let’s commit, this seems to be going well enough. Commit: initial Box handles strikes properly.

Now what do we want to do about a score that isn’t a strike? The small box on the screen should display the number of pins dropped. And we should certainly not accumulate any more rolls, if we’re the strike box. (Yes, we’ll probably rename this class soon.)

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

Here we should consume the 8 and ignore the others. Should we check the consumed return? I’m not sure. This is enough for now. Probably more than enough.

This makes the test pass but I am not fond of having all those ifs.

    def add_score(self, number):
        if not self._scores:
            self._scores = [number]
            return True
        if self._scores[0] < 10:
            return False
        if len(self._scores) < 3:
            self._scores.append(number)
        return len(self._scores) == 1

    def score(self):
        if not self._scores:
            return None
        if self._scores[0] == 10:
            return sum(self._scores) if len(self._scores) == 3 else 0
        return self._scores[0]

I went beyond my remit with the return of None, but it is part of the spec as I see it, because when we don’t have enough values we shouldn’t be returning zero, we should return None. Which means that the return of zero there is also wrong. Fix that. Tests are all passing. Commit: messy handling of non strike

Let’s add tests for the None.

    def test_strike_box_unsatisfied(self):
        box = Box()
        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

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

Commit: new tests

Now I really don’t like that code. It’s clear that the strike box has a number of states, perhaps these:

• Empty - nothing contained - unsatisfied
• Strike - initial roll is 10 - unsatisfied
• Strike 2 - strike and two rolls accumulated - unsatisfied
• Strike 3 - strike and three rolls accumulated - SATISFIED
• Non-strike - initial roll was not ten - SATISFIED

What if we had a flag for satisfied and used that instead of the length?

I think I want to draw a little state diagram here. And I’m thinking of having a second object inside the box. We’ll see.

I’m going to keep the current tests and throw away the methods. Oh, my, this went easily!

class Box:
    def __init__(self):
        self._scores = []
        self._state = self._empty

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

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

    def _empty(self, number):
        self._scores.append(number)
        if number == 10:
            self._state = self._strike_1
        else:
            self._state = self._satisfied
        return True

    def _strike_1(self, number):
        self._scores.append(number)
        self._state = self._strike_2
        return False

    def _strike_2(self, number):
        self._scores.append(number)
        self._state = self._satisfied
        return False

    def _satisfied(self, _number):
        return False

Full disclosure: the code above is slightly improved from the first cut: I added all the underbars that are in there, to make the state methods stand out. I just wanted to spare you a paste of that many lines that immediately changed.

How about a ternary here:

    def _empty(self, number):
        self._scores.append(number)
        self._state = self._strike_1 \
            if number == 10 \
            else self._satisfied
        return True

What I like about that is that it makes it clear that the two branches of the if are both setting state. What I do not like is that it’s a rather long line. We’ll live with it a while.

And this is a good time for this session to end. Let’s reflect and sum up.

Reflection

I don’t think I’ve even done a state solution to bowling before, so that is a pleasant result. While it’s a pretty simple diagram and code, it is a bit obscure to be setting a method name into a variable and then executing it. And after drawing the diagram, I was able to just type in the new version of the class and it worked as soon as I remembered to return the result from add_score.

I wonder if some renaming would make it more clear. Let me commit this before I forget: state-based version.

What iw we renamed _state to _next_action, and renamed the state methods like this:

class Box:
    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_first_bonus if number == 10 else self._satisfied
        return True

    def _record_first_bonus(self, number):
        self._scores.append(number)
        self._next_action = self._record_second_bonus
        return False

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

    def _satisfied(self, _number):
        return False

I think I like that better. The method names say what’s going on in that state. The other names named the state but didn’t give much info.

What will be next? I’m not sure in any detail. We’ll probably want to rename the Box class to something more specific and to create the second Box class, the one for spares. I think we’ll need to run into some way of indicating whether that box accepts any rolls at all. And presently we don’t even have access to any other boxes from the one we have. So I think it’ll get interesting.

But for now, we have a new way of approaching things, and we have written a nice little state machine. I am pleased with the morning. We’ll see how I feel about it when I come back to it.

See you then! And remember, resist fascism, and take care of one another!