Generator Function
Python Asteroids+Invaders on GitHub
Bill Wake suggested a generator function for my invader starting heights. I think this will be a keeper.
Bill Wake, @billwake@mastodon.online, suggested:
I see Python has generator functions - maybe you could yield 0, then have a loop that repeatedly returns 1-8.
I tried a little experiment in Pythonista (the iPad Python app) last night, and I think this idea will turn out to be a keeper.
We won’t do quite as Bill suggested, and I am definitely concerned about how we’ll get the thing bootstrapped but I think I have a plan. We’ll begin with some tests. Let me write the first one and then we’ll talk about what’s going on.
def test_simple_generator(self):
def g():
yield 0
y = 0
while True:
yield y + 1
y = (y + 1) % 4
gen = g()
assert next(gen) == 0
assert next(gen) == 1
assert next(gen) == 2
assert next(gen) == 3
assert next(gen) == 4
assert next(gen) == 1
Looking at g, imagine for a moment that the yield is a print. Then g will print 0, 1, 2, 3, 4, 1, 2, 3, 4 … forever.
The yield is the magic. I would say that it makes the function g into a coroutine, but Python’s terminology seems to be different. They call it a generator function. When we call gen = g(), what we get back is, if I have the terms right, a generator. And when we say next(gen) the generator runs to the next yield, then suspends, returning whatever is yielded. So instead of printing that sequence, we return it, one element at a time.
The test runs, so it is doing what I just described. Just exactly how it does it, I do not know, but I don’t really know just exactly anything, and since this clearly works, I like it.
Now for the real thing, I think I’d like it to return the actual starting y values in our game, not the original 8080 values. Recall that the 8080 values are defined in u.py:
INVADER_FIRST_START = 0x78
INVADER_STARTS = 0x60, 0x50, 0x48, 0x48, 0x48, 0x40, 0x40, 0x40
# 8080 values height = 256 = 0x100. Our screen is 4x that size
# and upside down.
Let’s test-drive a generator we might actually use. The test shell will look like this:
def test_y_generator(self):
def gen_y():
pass
y_generator = gen_y()
assert next(y_generator) == 1024 - 4*u.INVADER_FIRST_START
for i in range(8):
assert next(y_generator) == 1024 - 4*u.INVADER_STARTS[i]
assert next(y_generator) == 1024 - 4*u.INVADER_STARTS[0]
PyCharm knows that gen_y needs to include a yield, so there are squiggles all over the test, but when we’re done it will check that we return the actual y coordinate corresponding to INVADER_FIRST_START and then cycle the INVADER_STARTS values. I am confident that if it wraps back to zero it’ll be fine. We could of course code it to not be fine, but we’re not going to do that.
Now to write gen_y.
def test_y_generator(self):
def gen_y():
def convert(y_8080):
return 0x400 - 4*y_8080
yield convert(u.INVADER_FIRST_START)
index = 0
while True:
yield convert(u.INVADER_STARTS[index])
index = (index + 1) % len(u.INVADER_STARTS)
y_generator = gen_y()
assert next(y_generator) == 1024 - 4*u.INVADER_FIRST_START
for i in range(8):
assert next(y_generator) == 1024 - 4*u.INVADER_STARTS[i]
assert next(y_generator) == 1024 - 4*u.INVADER_STARTS[0]
As you can see, I put an inner function into gen_y to do the conversion to our coordinates, so this function is returning exactly the values that our InvaderFleet wants to have.
A Bit of Planning
So, how are we going to use this? Recall that we create a new InvaderFleet for every rack of invaders. We could reuse the instance, but I feel better going with a clean one rather than trying to clean a dirty one. So we’ll need to pass our generator to each new InvaderFleet, so that it can use it to get the y coordinate that it needs.
So how about this: InvaderFleet currently expects an integer parameter. We’ll belay that, which may break some tests, and change InvaderFleet so that when it is started with None, it creates the generator, and saves it in a member variable, and otherwise InvaderFleet receives a generator and saves that. Then it uses the generator to get its y coordinate, and when it’s time to make a new one, it passes the generator on to the new instance, where it will be ready to yield the next y coordinate. And so on, and so on, and scooby-dooby-doo.
I am inclined to treat this as a refactoring and just do the implementation, dealing with the tests as they break. One could argue that we should change the tests to have the new expectations and then change the code, so let’s at least look at that notion.
A quick glance twinges my conscience enough to decide to adjust the tests first. When we can do that, it’s surely better. And here, I think we can. We’ll discuss this a bit more down below.
Adjust the Tests
def test_initial_fleet_y(self):
fleet = InvaderFleet()
starting_y = fleet.origin.y
assert starting_y == 1024 - 4*u.INVADER_FIRST_START
def test_initial_fleet_y_given_0_parameter(self):
fleet = InvaderFleet(0)
starting_y = fleet.origin.y
assert starting_y == 1024 - 4*u.INVADER_STARTS[0]
def test_initial_fleet_y_given_8_parameter(self):
fleet = InvaderFleet(8)
starting_y = fleet.origin.y
assert starting_y == 1024 - 4*u.INVADER_STARTS[0]
def test_fleet_after_initial_is_0_fleet(self):
fleet = InvaderFleet()
assert fleet.origin.y == 1024 - 4*u.INVADER_FIRST_START
fleet = fleet.next_fleet()
assert fleet.origin.y == 1024 - 4*u.INVADER_STARTS[0]
fleet = fleet.next_fleet()
assert fleet.origin.y == 1024 - 4*u.INVADER_STARTS[1]
def test_fleet_starting_values_including_wrap(self):
fleet = InvaderFleet()
assert fleet.origin.y == 1024 - 4*u.INVADER_FIRST_START
for i in range(len(u.INVADER_STARTS)):
fleet = fleet.next_fleet()
assert fleet.origin.y == 1024 - 4*u.INVADER_STARTS[i]
fleet = fleet.next_fleet()
assert fleet.origin.y == 1024 - 4*u.INVADER_STARTS[0]
That’s most of them if not all. The ones that are passing numeric parameters will need to be changed. I expect that they will all work quickly except for the one with the 8. Anyway I’ll change those all to have no parameter for now.
I rename the two that needed explicit values and recast them so that they work now, and should work when we’re done:
def test_initial_fleet_y(self):
fleet = InvaderFleet()
starting_y = fleet.origin.y
assert starting_y == 1024 - 4*u.INVADER_FIRST_START
def test_second_fleet(self):
fleet = InvaderFleet()
fleet = fleet.next_fleet()
starting_y = fleet.origin.y
assert starting_y == 1024 - 4*u.INVADER_STARTS[0]
def test_fleet_wrap(self):
fleet = InvaderFleet()
fleet = fleet.next_fleet()
for i in range(len(u.INVADER_STARTS)):
fleet = fleet.next_fleet()
starting_y = fleet.origin.y
assert starting_y == 1024 - 4*u.INVADER_STARTS[0]
The tests are all green. So commit: recast tests to support generator as well as current scheme.
Put in the Generator
I plan to put the code in directly into __init__. That will not be attractive but it will be functional and then, once it works, we’ll refactor for a better arrangement. The current code is this:
class InvaderFleet(InvadersFlyer):
step = Vector2(8, 0)
down_step = Vector2(0, 32)
def __init__(self, start_index=None):
helper = OriginHelper.make_helper(start_index)
y = self.convert_y_coordinate(helper.starting_8080_y)
self.origin = Vector2(u.SCREEN_SIZE / 2 - 5 * 64, y)
self.next_index = helper.next_index
self.invader_group = InvaderGroup()
self.invader_group.position_all_invaders(self.origin)
self.direction = 1
self.step_origin()
It’s going to be bye-bye to my neat little helper classes. That’s OK and we’ll talk about that in the summary.
First, sketching intention, I think we’ll have something like this:
def __init__(self, generator=None):
if generator is None:
def generate_y():
yield 0
generator = generate_y()
self.y_generator = generator
y = next(self.y_generator)
self.origin = Vector2(u.SCREEN_SIZE / 2 - 5 * 64, y)
self.invader_group = InvaderGroup()
self.invader_group.position_all_invaders(self.origin)
self.direction = 1
self.step_origin()
def next_fleet(self):
return InvaderFleet(self.y_generator)
It just remains to put in the correct generate_y, and we have a suitable one in the tests. I’ll move it in.
This code passes all the tests:
def __init__(self, generator=None):
if generator is None:
def generate_y():
def convert(y_8080):
return 0x400 - 4*y_8080
yield convert(u.INVADER_FIRST_START)
index = 0
while True:
yield convert(u.INVADER_STARTS[index])
index = (index + 1) % len(u.INVADER_STARTS)
generator = generate_y()
self.y_generator = generator
y = next(self.y_generator)
self.origin = Vector2(u.SCREEN_SIZE / 2 - 5 * 64, y)
self.invader_group = InvaderGroup()
self.invader_group.position_all_invaders(self.origin)
self.direction = 1
self.step_origin()
def next_fleet(self):
return InvaderFleet(self.y_generator)
We can commit this. I am quite confident. I really wish it were easier to test in the game. I find a quick hack that kills all the invaders if I manage to shoot even one and the game works as advertised, with a caveat. We’ll talk about that. Commit: using new generator for y coordinates.
Now let’s clean that __init__ up a bit.
def __init__(self, generator=None):
self.y_generator = self.use_or_create(generator)
y = next(self.y_generator)
self.origin = Vector2(u.SCREEN_SIZE / 2 - 5 * 64, y)
self.invader_group = InvaderGroup()
self.invader_group.position_all_invaders(self.origin)
self.direction = 1
self.step_origin()
@staticmethod
def use_or_create(generator):
if generator:
return generator
else:
def generate_y():
def convert(y_8080):
return 0x400 - 4*y_8080
yield convert(u.INVADER_FIRST_START)
index = 0
while True:
yield convert(u.INVADER_STARTS[index])
index = (index + 1) % len(u.INVADER_STARTS)
return generate_y()
We are green again. Commit: factor out generator creation.
I think we can put that function anywhere. Possibly making it a first-class function in the file will be better:
def generate_y():
def convert(y_8080):
return 0x400 - 4 * y_8080
yield convert(u.INVADER_FIRST_START)
index = 0
while True:
yield convert(u.INVADER_STARTS[index])
index = (index + 1) % len(u.INVADER_STARTS)
class InvaderFleet(InvadersFlyer):
step = Vector2(8, 0)
down_step = Vector2(0, 32)
def __init__(self, generator=None):
self.y_generator = self.use_or_create(generator)
y = next(self.y_generator)
self.origin = Vector2(u.SCREEN_SIZE / 2 - 5 * 64, y)
self.invader_group = InvaderGroup()
self.invader_group.position_all_invaders(self.origin)
self.direction = 1
self.step_origin()
@staticmethod
def use_or_create(generator):
if generator:
return generator
else:
return generate_y()
Oh yes, much better. One more thing:
@staticmethod
def use_or_create(generator):
return generator if generator else generate_y()
Commit: tidying.
Remove the helper classes and their tests. Commit: remove Origin helper classes.
Reflection
This went marvelously. I am glad that I looked at the tests and took the occasion to improve them before changing the code, and of course I am sure I did absolutely the right thing when I wrote the two initial generator tests. They served to give me a solid place to stand to deal with just the generator aspects, first generating an values in a loop, and then generating actual y coordinates.
Having that in hand made it much more easy to put the capability into InvaderFleet: it was little more than copy-paste, see that the tests ran—first time, by the way—and then refactoring to make the code more neat, tidy, and clear.
Speaking of the tests. When we have decent tests, as in this case we did, we have the choice, when changing the design, between changing the tests that need it beforehand, or waiting until they break and fixing them after. Some of my colleagues might say that we should always change them, and that we should always have an accurate prediction of what is going to break when we make changes.
I am not so rigid about that. Quite often, I change the design. If the tests happen to be written at the right level, nothing will break. Since my tests are often a bit more invasive, some of them sometimes do break. Generally, the fix is just to align them with the new interface or whatever is changing. Once in a while a test will surprise me, generally indicating that my plan is flawed.
If I had to say what is better, I might say that inspecting all the tests and making sure they are up to snuff against a new set of changes is probably best. If I had to say what is faster, I suspect that letting them break and fixing them is probably faster. But it is more risky, because fixing them up sometimes means accepting a new value and sometimes it shouldn’t have been accepted. So I think I have a slightly better (worse) chance of a defect when I just let them break and then fix them up.
I’m not perfect and I don’t always do the thing that is somehow, somewhere, defined as best, or as “the way”. Instead, I do what seems right, and I pay attention to what happens. When something happens that I don’t like, I try to think about it, and lean a bit in the direction of doing better next time.
By the book is not always right. The book is how we teach the idea, but it is not how the reader learns the idea. She learns the idea by doing the work, using our examples, thinking about our advice, and observing what really happens to her as she does the work.
In this case, I was able to very quickly adjust the tests to a better state and they kept going green, which gave me confidence and a little charge of serotonin or something like it.
Overall, it went very smoothly. I am well pleased.
I think there is one flaw. The generator that we wrote and tested in the tests is not the same one that we use in the actual code. It would be wise to have the tests testing the one we actually use. I’ll make a note of that.
Summary
Let’s take one more look at the larger picture:
def __init__(self, generator=None):
self.y_generator = self.use_or_create(generator)
y = next(self.y_generator)
self.origin = Vector2(u.SCREEN_SIZE / 2 - 5 * 64, y)
self.invader_group = InvaderGroup()
self.invader_group.position_all_invaders(self.origin)
self.direction = 1
self.step_origin()
@staticmethod
def use_or_create(generator):
return generator if generator else generate_y()
def next_fleet(self):
return InvaderFleet(self.y_generator)
Take a look at #294 for a reminder of what we used to have, with more complicated code and those three little helper classes carrying little more weight than our simple generator function does.
Am I sad to see those helpers go? Not a bit: this is better by my lights. Am I sorry I did those helper classes? Not a bit: they were better than what I had before, again by my lights. And I got a bit of good practice in while doing them.
But let’s talk about my lights for a moment, and yours.
I think the helper classes, especially the way they swapped in and out, was a bit deeper in the bag of tricks than the open code we had before. And there’s no doubt that the generator function is a bit deeper still. We always want to work at the shallow end of the bag of tricks, digging deeper only rarely, never much further than the team can handle.
At the same time, the team wants to grow its capabilities, so we will sometimes try something new and deeper down. We are wise to try it with tests, and to work as a team, or at least with pairs, so that the understanding spreads as quickly as possible.
In essence, our generator function produces a table of indefinitely length, starting with that one special starting value, and then containing the eight standard decreasing values, over and over. As a generator, it never produces the whole table, just the next element.
The part that I personally find most odd is that we define the generator function, and then call it, once, and what we get back is not the first value, but a thing such that when we subsequently say next(thing) it then produces the yield values.
We see that oddity in this method:
@staticmethod
def use_or_create(generator):
return generator if generator else generate_y()
We have defined the function generate_y long before, up at the top of the file. But when we are about to use it for the first time, we call it, once, and return that as the actual generator.
I find that odd and want to read more about it. But between the tests and the code, I am confident that it’s working as it should. I’d just like a bit more understanding.
So is the generator on the hairy edge of too deep in the bag? Maybe so … but its improvement of the code is well worth it, to me and the rest of my team (also me).
What do you think? Is this a change you would make, by your lights? What questions are you left with? What comments or concerns?
See you next time!