Still Sniped
An imaginary chat with my imaginary customer tells us what our next few steps need to do. Also: Resist fascism, and take care of one another.
I was talking with myself about what we need next in our bowling exercise, and the following ideas were among the topics:
-
My next “internal” step, when I get to take it, will probably be to compute the correct single-frame score under all circumstances.
-
Another step will be to accumulate the preceding single-frame scores into each actual frame score.
-
The customer is prepared to read my pytest tests to help ensure that my code is getting the right answer.
-
The customer feels that he would like to see the game display an extra line of information, showing the actual rolls as provided as input, ideally parsed out to be underneath the frame they’re assigned to. It is possible that we will retain that ability, which might be helpful in production, when maintenance people are diagnosing troubles.
-
We think we might like to have an “acceptance test” mode where predefined sequences are scored, perhaps all at once or perhaps even roll by roll, upon the customer typing an enter. We might even want the customer to be able to enter rolls manually, although I have indicated that that will be an expensive feature.
Let’s get to it. I think I’ll start by extending the view vertically, to provide room for the rolls. My guess is that I’ll put them under the frame drawings.
That requires me to draw a bottom line on the frame boxes. We were using the window boundary to serve for that. No problem! I rather quickly have this, for example. The final draw line is added:
def draw(self, game):
x = self.f * game.FRAME_SIZE
mark_line = game.MARK_LINE
pygame.draw.line(game.screen, game.WHITE, (x, 0), (x, game.GAME_Y))
pygame.draw.line(game.screen, game.WHITE, (x, mark_line), (x - mark_line, mark_line))
pygame.draw.line(game.screen, game.WHITE, (x - mark_line, mark_line), (x - mark_line, 0))
pygame.draw.line(game.screen, game.WHITE, (x - game.FRAME_SIZE, game.GAME_Y), (x, game.GAME_Y))
self.draw_frame_total(game)
And the picture looks like this:
As I was doing all this, I noticed an odd thing that I’ve done, and I don’t recall why I did it. The frame view instances are created with their frame numbers set from 1 to 10, not 0 to 9 as one might have expected. The effect is to cause all those subtractions in the drawing. I think we should fix that, because a 1-based index is sure to confuse us.
def __init__(self):
pygame.init()
self.running = False
self.screen = pygame.display.set_mode(self.WINDOW_SIZE)
pygame.display.set_caption('Bowling')
self.frames: List[Union[NormalFrameView, TenthFrameView]]
= [NormalFrameView(f) for f in range(0, 9)]
self.frames.append(TenthFrameView())
I changed the for range from (1, 10) to (0, 9). Now the display will be wrong. Would you like to see it?
Easily fixed. A long pause ensues. Well, I say “easily”. The changes were small and simple but it’s a bit tricky to get things just right. I’ve got the NormalFrameView right now:
def draw(self, game):
x_left = self.f * game.FRAME_SIZE
x_right = x_left + game.FRAME_SIZE
mark_line = game.MARK_LINE
pygame.draw.line(game.screen, game.WHITE, (x_right, 0), (x_right, game.GAME_Y))
pygame.draw.line(game.screen, game.WHITE, (x_right - mark_line, mark_line), (x_right , mark_line))
pygame.draw.line(game.screen, game.WHITE, (x_right - mark_line, mark_line), (x_right - mark_line, 0))
pygame.draw.line(game.screen, game.WHITE, (x_left, game.GAME_Y), (x_right, game.GAME_Y))
self.draw_frame_total(game)
def draw_frame_total(self, game):
total = 121
font = pygame.font.Font(game.BASE_FONT, 30)
font.set_bold(True)
text = font.render(str(total), True, game.FOREGROUND, game.BACKGROUND)
text_rect = text.get_rect()
text_rect.centerx = self.f * game.FRAME_SIZE + game.FRAME_SIZE / 2
text_rect.centery = game.GAME_Y - 20
game.screen.blit(text, text_rect)
That’s all very ad hoc and should be generalized based on information we don’t have, like the actual characteristics of the display. I think we can do better, and so I’ve put it on a card, but we are here to please the customer. The customer asks, where is the number for the tenth frame? I reply that I just haven’t done it yet, but I want to see it so I agree to do it now.
It’s well past time to commit all this stuff: prepare display for showing rolls.
I agree with myself that a good next step would be to work on showing actual scores in the frames, beginning to do the underlying objects that will do the calculations. My rough plan is that there will be a Frame object associated with each FrameView object, and the FrameView will ask the Frame to provide the fields needed for the display. I haven’t decided who will say that the mark should be a slash or X but I am inclined to have the Frame do it if the code agrees.
I plan to provide a single growing list of rolls, and to have each Frame know where in the list its first roll is. I haven’t worked out details of all this. Let’s use some tests to make it happen.
I use this test:
def test_scorekeeper(self):
s = Scorekeeper()
assert s.rolls == []
assert len(s.frames) == 10
To drive out this code, which is more than the test requires, but I am not a fanatic about that arbitrary rule.
class Frame:
def __init__(self, rolls):
self.rolls = rolls
class Scorekeeper:
def __init__(self):
self.rolls = []
self.frames = [Frame(self.rolls) for _ in range(10)]
My theory of all this is that rolls will arrive willy-nilly, and upon each one arriving, we’ll parse the rolls we have into the frames, and frames that have enough rolls will be able to compute their frame score, and frames that are not ready will return None for that value. The Frame will also be able to provide the other fields we need in the FrameViews, in due time.
I plan to use exceptions throughout the Frame, at least until I decide that I don’t like doing that.
def test_open_frame(self):
s = Scorekeeper()
s.roll(5)
s.roll(4)
assert s.frame(0).score == 9
That permits me to write this:
def roll(self, value):
self.rolls.append(value)
self.update_frames()
def update_frames(self):
start = 0
for frame in self.frames:
frame.start = start
start = frame.next_start()
Frame does not know how to cooperate with this, so I do this much and then discover that I want an easier test.
class Scorekeeper:
def __init__(self):
self.rolls = []
self.frames = [Frame(self.rolls) for _ in range(10)]
def frame(self, n):
return self.frames[n]
def roll(self, value):
self.rolls.append(value)
self.update_frames()
def update_frames(self):
start = 0
for frame in self.frames:
frame.start = start
start = frame.next_start()
class Frame:
def __init__(self, rolls):
self.rolls = rolls
self.start = None
def next_start(self):
if self.start is None:
return None
first_roll = self.rolls[self.start]
desired_rolls = 1 if first_roll == 10 else 2
if self.start + desired_rolls < len(self.rolls):
return self.start + desired_rolls
return None
The idea here is that we’ll fill in start if there are enough rolls to allow the frame to see its first roll, and not otherwise. I did a simpler test to get this far:
def test_open_frame(self):
s = Scorekeeper()
s.roll(5)
s.roll(4)
assert s.frame(0).start == 0
assert s.frame(1).start is None
That’s passing. I’m not really confident in this code but the rules ask me to commit on green, so: scorekeeper and frame working so far.
Make a harder test:
def test_open_frame_plus_a_roll(self):
s = Scorekeeper()
s.roll(5)
s.roll(4)
s.roll(6)
assert s.frame(0).start == 0
assert s.frame(1).start == 2
That passes, as I hoped and expected at about p = 0.75. Commit: further testing of next_start.
I really think this works. Let’s start testing frame_score, the method that will return the frame’s own score (not the accumulated score).
def test_first_frame_score(self):
s = Scorekeeper()
s.roll(5)
s.roll(4)
assert s.frame(0).frame_score() == 9
Now I plan to write as much of the code as I understand, which, again, is more than my test demands. Let’s see if violating the “code only what the test demands” rule bites me.
def frame_score(self):
try:
return self._frame_score()
except Exception:
return None
def _frame_score(self):
s = self.start
rolls = self.rolls
first_roll = rolls[s]
if first_roll == 10:
return 10 + rolls[s+1] + rolls[s+2]
second_roll = rolls[s+1]
if (two := first_roll + second_roll) != 10:
return two
return two + rolls[s+2]
If anything goes wrong in _frame_score(), we’ll return None as advertised. The only thing that can go wrong is an access to a roll that hasn’t appeared yet.
I am not more than maybe 34% proud about _frame_score() but I think it’s actually the whole deal. We’ll add a bunch of tests now, to make sure.
def test_strike_scoring(self):
s = Scorekeeper()
s.roll(10)
assert s.frame(0).frame_score() is None
s.roll(9)
assert s.frame(0).frame_score() is None
s.roll(8)
assert s.frame(0).frame_score() == 27
I forgot to commit. That was probably due to an unacknowledged lack of confidence. I’ll do so now: commit: open frame and strike scoring tested correct.
def test_spare_scoring(self):
s = Scorekeeper()
s.roll(7)
assert s.frame(0).frame_score() is None
s.roll(3)
assert s.frame(0).frame_score() is None
s.roll(8)
assert s.frame(0).frame_score() == 18
Green, after fixing the test to actually have a spare. Commit: spare test passes.
I think we should have at least one more test, since all these are checking just the first frame.
Let’s do this one:
def test_alternating(self):
s = Scorekeeper()
for frame in range(5):
s.roll(10)
s.roll(9)
s.roll(1)
s.roll(10)
total = sum(frame.frame_score() for frame in s.frames)
assert total == 200
Years ago, I was doing some XP training in Omaha, with Jeff Langr, and we did the bowling demo. One of the class members was a woman’s bowling champion and when we asked for a test that might fail, she offered this interesting fact:
Any game of alternating strikes and spares will give a score of 200. Since then, I’ve generally told that story and used that as a test in the bowling demo.
And it’s passing. Commit: alternating strike/spare scores 200 as expected.
This is a good place to pause and sum up, I think.
Summary
We’ve provided a “debug” space on the screen for our customer to view the input rolls for a manual check of results. I expect these to show us nothing new but I can understand wanting to see something that looks like the rolls and the corresponding frame display. I might want to help my customer gain confidence from the tests that I write, but what convinces me, given my understanding of the code, is not necessarily the same as what might convince a normal human.
On some of my changes today, I went beyond the strict remit of the test I was making work. Wise heads often advise us “only write enough code to pass the test, no more and no less”. I think that’s good practice, and it helps us take small steps. I also think that we are grownups here, and we can decide not to follow some rule that we found in a book somewhere.
However, you’ll notice that after writing more code than my tests called for, I wrote further tests that exercised all the code I had written. In this case, I had the code in my head, or nearly so, so I wrote it and then tested it. Doing it in smaller steps would have felt artificial to me.
And it worked. Good move? Great move? Poor move and I got lucky? You get to decide what you think, and to guide your own work accordingly.
As things stand we have code that could benefit from some improvement.
def next_start(self):
if self.start is None:
return None
first_roll = self.rolls[self.start]
desired_rolls = 1 if first_roll == 10 else 2
if self.start + desired_rolls < len(self.rolls):
return self.start + desired_rolls
return None
def _frame_score(self):
s = self.start
rolls = self.rolls
first_roll = rolls[s]
if first_roll == 10:
return 10 + rolls[s+1] + rolls[s+2]
second_roll = rolls[s+1]
if (two := first_roll + second_roll) != 10:
return two
return two + rolls[s+2]
There’s almost some duplication in what’s above. That might want to be improved. The _frame_score could be more expressive. It’s doing strike, open, spare, in that order, but it doesn’t exactly say that.
I don’t love that next_start starts by returning a None and ends that way, with another return in the middle. I think we could improve that as well.
I kind of like the repeated update of the start locations upon each new roll, although it is redundant. An optimization might be to remember how many are not initialized, or to skip the ones that are initialized … but I kind of suspect that would be less clear than it is now.
Finally, I’d like to improve the drawing substantially, because it is far from clear which draw is drawing what element of the picture, and the arithmetic is hard to grok. And there is great similarity between the NormalFrameView and TenthFrameView code. We might be able to reduce that duplication, but I think it would come at the expense of an abstract superclass or some kind of bodged together class that handles both situations.
Maybe we can find a helper class that they can both use. That might work out nicely.
All in all, substantial progress and the code isn’t terrible. It can be better, and it will be, but we have a nice little addition to the package. See you next time! And resist fascism, and take care of one another!