Scoring details aren’t quite right. Long, tedious, more correct results, but I don’t like the code much. Bear got a taste of me. Also: Resist fascism, and take care of one another.

On bowl.com I find instructions for scoring, perhaps aimed at youth. Perfect, I was once a youth, if I correctly recall.

I note immediately that they draw two small squares, right justified, in the first nine frames, and three in the tenth. They are also showing the tenth frame wider than the other nine. I don’t love that idea and my customer side will agree with me, I think. I do find some printable score sheets where all the frames are in the same size large square, so that’s probably OK.

I learn that if in the tenth frame you score a spare with your first two rolls, you mark that as usual with the number of pins in the first, and the slash in the second. They show a dash or underbar for a roll that gets no pins, not a zero.

They show the X for a strike in the first square, not the second square, of a normal frame. Then they leave the right square blank. Our code marks the strike in the rightmost square, I believe.

So we do need some revision here. Let’s do some TDD, and let’s define some terms along the way. Let’s call the three small squares in the tenth frame left, center, and right. And then in the normal frames, we’ll call the two squares … tentatively … center and right. We may revise this notion, depending on how things shape up. For now, center and right for normal frames, left, center, and right for tenth frame.

Warning
I do some tests and code and then roll back. To skip to the rollback, click here.

We have some tests for the marking already:

    def test_mark(self):
        s = Scorekeeper()
        s.roll(10)
        s.roll(6)
        s.roll(4)
        s.roll(3)
        s.roll(2)
        assert s.frame(0).mark() == 'X'
        assert s.frame(1).mark() == '/'
        assert s.frame(2).mark() == '2'
        assert s.frame(3).mark() == ''

    def test_first_roll(self):
        s = Scorekeeper()
        s.roll(10)
        s.roll(6)
        assert s.frame(0).first_roll() == 'X'
        assert s.frame(1).first_roll() == '6'

    def test_tenth_frame(self):
        s = Scorekeeper()
        s.roll(1)
        s.roll(2)
        s.roll(3)
        f = s.frame(0)
        assert f.tenth_frame(0) == '1'
        assert f.tenth_frame(1) == '2'
        assert f.tenth_frame(2) == '3'

    def test_tenth_middle(self):
        s = Scorekeeper()
        s.roll(0)
        s.roll(0)
        s.roll(7)
        s.roll(3)
        f = s.frame(1)
        assert f.tenth_frame(1) == '/'

I think I’ll just ignore those and do new ones, rather than try to revise those. Let’s posit, for now, some new methods on Frame.

I write this test:

    def test_strike_frame(self):
        s = Scorekeeper()
        s.roll(10)
        f: Frame = s.frame(0)
        assert f.center() == 'X'
        assert f.right() == ""

It seems to me that a frame might usefully know its frame number, or at least whether it is the tenth frame.

class Frame:
    def __init__(self, frame_number, rolls, previous_frame):
        self.frame_number = frame_number
        self.rolls = rolls
        self.previous_frame = previous_frame
        self.start = None

Speculative, I grant, but I want to do this:

    def center(self):
        if self.frame_number < 10:
            return self.first_roll()

    def right(self):
        if self.frame_number < 10:
            if self.center() == 'X':
                return ''

That passes my first test. Add another:

    def test_spare_frame(self):
        s = Scorekeeper()
        s.roll(6)
        s.roll(4)
        f: Frame = s.frame(0)
        assert f.center() == '6'
        assert f.right() == '/'

OK but I have a bad start, relying on first_roll. Let’s inline it and then proceed.

I make both tests work with this:

    def safe_value(self, roll):
        try:
            return self.rolls[self.start + roll]
        except Exception:
            return None

    def safe_sum(self, rolls):
        return sum(self.safe_value(roll) for roll in rolls)

    def center(self):
        if self.frame_number < 10:
            roll = self.safe_value(0)
            if roll is None:
                return ''
            elif roll == 10:
                return 'X'
            else:
                return str(roll)

    def right(self):
        if self.frame_number < 10:
            if self.center() == 'X':
                return ''
            elif self.safe_sum([0, 1]) == 10:
                return '/'
            else:
                return str(self.safe_value(1))

I don’t think safe_sum is doing what I want. Let’s prove it.

    def test_safe_sum(self):
        s = Scorekeeper()
        s.roll(6)
        f: Frame = s.frame(0)
        assert f.safe_sum([0, 1]) is None

That fails. I am not happy with where we are. Tempted to roll back, but not quite yet, because there are parts of this that I like.

I have the spare test and this new one working:

    def test_open_frame(self):
        s = Scorekeeper()
        s.roll(6)
        s.roll(3)
        f: Frame = s.frame(0)
        assert f.center() == '6'
        assert f.right() == '3'

With this code:


    def safe_sum(self, *rolls):
        try:
            return sum(self.rolls[self.start+roll] for roll in rolls)
        except Exception:
            return None

    def safe_value(self, roll):
        return self.safe_sum(roll)

    def center(self):
        if self.frame_number < 10:
            roll = self.safe_value(0)
            if roll is None:
                return ''
            elif roll == 10:
                return 'X'
            else:
                return str(roll)

    def right(self):
        if self.frame_number < 10:
            if self.center() == 'X':
                return ''
            elif self.safe_sum(0, 1) == 10:
                return '/'
            else:
                return str(self.safe_value(1))

I’m still not loving this code. I think it may get worse when we do the tenth frame. I’ll not commit it yet. Let’s do some tenth-frame work:

    def test_tenth_frame_spare_strike(self):
        s = Scorekeeper()
        for _ in range(1,11):
            s.roll(0)
            s.roll(0)
        s.roll(6)
        s.roll(4)
        s.roll(10)
        f = s.frame(9)
        assert f.frame_number == 10
        assert f.left() == '6'
        assert f.center() == '/'
        assert f.right() == 'X'

I can see what I’m going to have to do here, and it’s awful. I need a new method left, and I need to add cases for frame_number 10 in the other two methods. Let’s belay all this and take another run at it.

Roll Back

I might have rolled back sooner, but I wanted to push the ideas a bit further. However, it’s still not getting better. Time to roll back, and I did.

I see at least two ways to proceed:

  1. Just fix the existing code up to do what we want. I think it is nearly getting the right answers, except possibly for a spare mark in the middle of the tenth frame.

  2. Try to express all the desired results in a more compact form, such as a method that returns all three cells for any frame. I’d like to try structural pattern matching for this.

Let’s explore #1 a bit. First, let’s draw the second box in the lower-numbered frames. We have this:

    def draw_common_elements(self, game):
        x_left = self.f * game.FRAME_SIZE
        x_right = x_left + game.FRAME_SIZE
        mark_line = game.MARK_LINE
        self.vh.white_lines([(x_right, 0), (x_right, game.GAME_Y)])
        self.vh.white_lines([(x_left, game.GAME_Y), (x_right, game.GAME_Y)])
        self.vh.draw_frame_total(self.f, self.frame)
        self.vh.small_box(x_right, mark_line, 1)
        if self.f == 9:
            self.vh.small_box(x_right, mark_line, 2)
            self.vh.small_box(x_right, mark_line, 3)

Thanks to our work yesterday, we can add the box by just moving one line up:

    def draw_common_elements(self, game):
        x_left = self.f * game.FRAME_SIZE
        x_right = x_left + game.FRAME_SIZE
        mark_line = game.MARK_LINE
        self.vh.white_lines([(x_right, 0), (x_right, game.GAME_Y)])
        self.vh.white_lines([(x_left, game.GAME_Y), (x_right, game.GAME_Y)])
        self.vh.draw_frame_total(self.f, self.frame)
        self.vh.small_box(x_right, mark_line, 1)
        self.vh.small_box(x_right, mark_line, 2)
        if self.f == 9:
            self.vh.small_box(x_right, mark_line, 3)

And I test to see what we display for our interesting cases:

score sheet showing need for dash on open and slash in rightmost box

We really just need a dash for zero, and a slash in the last square if the rolls are strike, n, 10-n. Maybe just fixing the existing code makes sense.

I change this:

    def _first_roll(self):
        s = self.start
        if self.strike((r := self.rolls[s])):
            return 'X'
        else:
            return '-' if r == 0 else str(r)

The final return used to just be str(r).

And I need to change this:

    def _mark(self):
        s = self.start
        rolls = self.rolls
        if self.strike(rolls[s]):
            return 'X'
        elif rolls[s] + rolls[s + 1] == 10:
            return '/'
        else:
            return str(rolls[s+1])

To this:

    def _mark(self):
        s = self.start
        rolls = self.rolls
        if self.strike(rolls[s]):
            return 'X'
        elif rolls[s] + (r2 := rolls[s + 1]) == 10:
            return '/'
        else:
            return '-' if r2 == 0 else str(r2)

That should give me the dashes that I need, and it does. We’ll look at them in the final picture coming up soon.

Now the final slash if the bonus rolls are a spare. Drawing the tenth frame goes like this:

class NormalFrameView:
    def draw(self, game):
        if self.f < 9:
            self.draw_1_thru_9(game)
        else:
            self.draw_10(game)

    def draw_10(self, game):
        self.draw_common_elements(game)
        self.draw_tenth_frame(0)
        self.draw_tenth_frame(1)
        self.draw_tenth_frame(2)

    def draw_tenth_frame(self, n):
        mark = self.frame.tenth_frame(n)
        step = 2*n + 1
        self.vh.draw_in_box(self.f, mark, step)

class Frame:
    def tenth_frame(self, n):
        try:
            return self._tenth_frame(n)
        except Exception:
            return ''

    def _tenth_frame(self, n):
        index = self.start + n
        roll = self.rolls[index]
        if n == 1:
            return self.middle_cell(roll)
        else:
            return self.outer_cells(roll)

    def middle_cell(self, roll):
        if self.tenth_frame_spare(roll):
            return '/'
        else:
            return str(roll)

    def tenth_frame_spare(self, roll):
        return roll + self.rolls[self.start] == 10

    def outer_cells(self, roll):
        if self.strike(roll):
            return 'X'
        else:
            return str(roll)

    def strike(self, roll):
        return roll == 10

That’s rather messy. Let’s just break out all three cases for now.

And how about writing some tests for it?

    def test_tenth(self):
        f = self.get_tenth_frame(10, 6, 4)
        left, center, right = self.get_boxes(f)
        assert left == 'X'
        assert center == '6'
        assert right == '/'

No, this is better yet:

    def test_tenth(self):
        f = self.get_tenth_frame(10, 6, 4)
        boxes = self.get_boxes(f)
        assert boxes == 'X6/'

If it’s going to be easy it might as well be really easy.

I need those two helper methods:

    def get_tenth_frame(self, r1, r2, r3):
        s = Scorekeeper()
        for _frame in range(1,10):
            s.roll(0)
            s.roll(0)
        s.roll(r1)
        s.roll(r2)
        s.roll(r3)
        return s.frame(9)

    def get_boxes(self, f):
        return ''.join([
            f.tenth_frame(0),
            f.tenth_frame(1),
            f.tenth_frame(2)
        ])

Test fails, I hope saying ‘X64’. Yes!

Expected :'X6/'
Actual   :'X64'

Now then, let’s see about fixing it. Here’s our base method:

    def _tenth_frame(self, n):
        index = self.start + n
        roll = self.rolls[index]
        if n == 1:
            return self.middle_cell(roll)
        else:
            return self.outer_cells(roll)

Let’s just break out all three cases:

    def _tenth_frame(self, n):
        index = self.start + n
        roll = self.rolls[index]
        if n == 0:
            return self.outer_cells(roll)
        elif n == 1:
            return self.middle_cell(roll)
        else:
            prev = self.rolls[index-1]
            if prev != 10 and prev + roll == 10:
                return '/'
            else:
                return '-' if roll == 0 else str(roll)

    def middle_cell(self, roll):
        if self.tenth_frame_spare(roll):
            return '/'
        else:
            return '-' if roll == 0 else str(roll)

    def tenth_frame_spare(self, roll):
        return roll + self.rolls[self.start] == 10

    def outer_cells(self, roll):
        if self.strike(roll):
            return 'X'
        else:
            return '-' if roll == 0 else str(roll)

    def strike(self, roll):
        return roll == 10

I notice that outer_cells is really just left_cell now. And we have lots of those conditionals to replace 0 with -. Fixing both:

    def _tenth_frame(self, n):
        index = self.start + n
        roll = self.rolls[index]
        if n == 0:
            return self.left_cell(roll)
        elif n == 1:
            return self.middle_cell(roll)
        else:
            prev = self.rolls[index-1]
            if prev != 10 and prev + roll == 10:
                return '/'
            else:
                return self.dash_for_zero(roll)

    def left_cell(self, roll):
        if self.strike(roll):
            return 'X'
        else:
            return self.dash_for_zero(roll)

    def dash_for_zero(self, roll):
        return '-' if roll == 0 else str(roll)

Extract method for the last bit in _tenth_frame. PyCharm can’t figure out what I want. I’ll just do it.

    def _tenth_frame(self, n):
        index = self.start + n
        roll = self.rolls[index]
        if n == 0:
            return self.left_cell(roll)
        elif n == 1:
            return self.middle_cell(roll)
        else:
            return self.right_cell(roll)

    def left_cell(self, roll):
        if self.strike(roll):
            return 'X'
        else:
            return self.dash_for_zero(roll)

    def middle_cell(self, roll):
        if self.tenth_frame_spare(roll):
            return '/'
        else:
            return self.dash_for_zero(roll)

    def right_cell(self, third_roll):
        second_roll = self.rolls[self.start + 1]
        if second_roll != 10 and second_roll + third_roll == 10:
            return '/'
        else:
            return self.dash_for_zero(third_roll)

Let’s add some tenth-frame tests. I’m glad I decided to do that. This fails:

        f = self.get_tenth_frame(10, 0, 10)
        assert self.get_boxes(f) == 'X-/'

That’s returning ‘X//’. Center is wrong. Here are my tests now:

    def test_tenth(self):
        f = self.get_tenth_frame(10, 6, 4)
        boxes = self.get_boxes(f)
        assert boxes == 'X6/'
        f = self.get_tenth_frame(10, 0, 10)
        assert self.get_boxes(f) == 'X-/'
        f = self.get_tenth_frame(0, 10, 10)
        assert self.get_boxes(f) == '-/X'

And the code, which I do not like:

    def _tenth_frame(self, n):
        index = self.start + n
        roll = self.rolls[index]
        if n == 0:
            return self.left_cell(roll)
        elif n == 1:
            return self.middle_cell(roll)
        else:
            return self.right_cell(roll)

    def left_cell(self, roll):
        if self.strike(roll):
            return 'X'
        else:
            return self.dash_for_zero(roll)

    def middle_cell(self, second_roll):
        first_roll = self.rolls[self.start]
        if first_roll != 10 and first_roll + second_roll == 10:
            return '/'
        else:
            return self.dash_for_zero(second_roll)

    def right_cell(self, third_roll):
        second_roll = self.rolls[self.start + 1]
        if second_roll == 10 and third_roll == 10:
            return 'X'
        elif second_roll != 10 and second_roll + third_roll == 10:
            return '/'
        else:
            return self.dash_for_zero(third_roll)

    def dash_for_zero(self, roll):
        return '-' if roll == 0 else str(roll)

    def tenth_frame_spare(self, roll):
        return roll + self.rolls[self.start] == 10

    def strike(self, roll):
        return roll == 10

Let’s see what we can simplify here. I think inlining a bit may help as well.

This is getting worse and worse, although I seem to be progressing. I found another case that failed:

        f = self.get_tenth_frame(10, 10, 10)
        assert self.get_boxes(f) == 'XXX'

That’s fixed with this:

    def left_cell(self, roll):
        return 'X' if roll == 10 else self.dash_for_zero(roll)

    def middle_cell(self, second_roll):
        first_roll = self.rolls[self.start]
        if first_roll == 10 and second_roll == 10:
            return 'X'
        elif first_roll != 10 and first_roll + second_roll == 10:
            return '/'
        else:
            return self.dash_for_zero(second_roll)

    def right_cell(self, third_roll):
        second_roll = self.rolls[self.start + 1]
        if second_roll == 10 and third_roll == 10:
            return 'X'
        elif second_roll != 10 and second_roll + third_roll == 10:
            return '/'
        else:
            return self.dash_for_zero(third_roll)

Now we have some duplication. Again PyCharm can’t extract. I’ll do it by hand.

I do a bit of tidying and wind up here:

    def tenth_frame(self, n):
        try:
            return self._tenth_frame(n)
        except Exception:
            return ''

    def _tenth_frame(self, n):
        roll = self.rolls[(self.start + n)]
        match n:
            case 0:
                return self.left_cell(roll)
            case 1:
                return self.middle_cell(roll)
            case _:
                return self.right_cell(roll)

    def left_cell(self, roll):
        return 'X' if self.strike(roll) else self.dash_for_zero(roll)

    def middle_cell(self, second_roll):
        first_roll = self.rolls[self.start]
        return self.score_second_of_two_rolls(first_roll, second_roll)

    def right_cell(self, third_roll):
        second_roll = self.rolls[self.start + 1]
        return self.score_second_of_two_rolls(second_roll, third_roll)

    def score_second_of_two_rolls(self, first_roll, second_roll):
        if self.strike(first_roll) and self.strike(second_roll):
            return 'X'
        elif self.spare(first_roll, second_roll):
            return '/'
        else:
            return self.dash_for_zero(second_roll)

    def strike(self, roll):
        return roll == 10

    def spare(self, first_roll, second_roll):
        return not self.strike(first_roll) and first_roll + second_roll == 10

    def dash_for_zero(self, roll):
        return '-' if roll == 0 else str(roll)

There’s not much duplication here, but it seems like a lot. I’m tired now, and out of good ideas. Let’s sum up.

Summary

The biggest question I am asking myself now is whether some of the past time might have been better spent doing some sketching of patterns and a bit of thinking without coding. Quite possibly it would have, but we’ll never know. I do plan to do that sketching and thinking later today, for next time.

I forgot that the strike goes in the first box in normal frames, until just now. Sorry. Next time better.

We actually had several changes that we needed to make, and we have successfully made the ones we tried. Except for marking the strike in the first box rather than the second, I think we’re returning the desired results for all the boxes, with dashes for zero, and X and / appearing properly for strikes and spares, including in the tenth frame, which wasn’t quite right before.

But I don’t like the code. My intuition tells me there is a simpler way that I just haven’t found yet. But we’re green and have better results. Commit: added dash for 0 and proper marks in tenth frame. Better idea needed.

Sometimes the bear gets a taste of you. See you next time: remember to resist fascism, and take care of one another.