Python Asteroids+Invaders on GitHub

Let’s look today at three classes that are similar but different. Make similar code more similar. Soft, gentle, geek joy.

It’s not the first time that I have noticed that InvaderPlayer, RobotPlayer, and ReservePlayer are very similar, but yesterday, since I had occasion to work on all three of them, their similarity came to mind quite strongly. Today, let’s examine the three classes and see whether they might need a bit of improvement.

Historically, we started with InvaderPlayer, which is a very active object, as it represents the human player of the game. Later, we added ReservePlayer, which is a relatively inactive object, just serving as a marker for how many additional lives you have left, down at the bottom of the screen. Finally, quite recently, we added the RobotPlayer, used in “attract mode”. It has logic to drive itself around the screen and fire shots, as if someone was playing the game.

As things stand now, all three of these objects draw themselves with exactly the same sprite, and each one of them can explode as needed.

Initial speculation …

Even without looking at the code, just thinking about how these things are alike and different, we can speculate about how one might consolidate their common elements and break out the differences. There are really only a few ways to do that.

We could use a common delegate for the drawing. We have the Sprite object and the Spritely interface, and that would certainly allow us to share most of the code for drawing. (If my memory is correct, two of the three classes are Spritely, and one is not.)

We could have a common superclass with other common bits that aren’t covered by Spritely or their other common superclass, InvadersFlyer. Then we could subclass to provide the unique behavior. We would surely use implementation inheritance, because it’s a legitimate element of our bag of tricks, though we prefer to use it sparingly.

We might find other delegates to deal with motion: one of them does not move, one is controlled by keys, one is controlled by code. Perhaps there could be one Player object and three drivers.

Let’s get concrete.

But enough hanging out in the coffee room speculating. Let’s look at these three objects as we design. We’ll compare them chunk by chunk, looking at the code that handles some aspect and comparing. Let’s start with object creation:

class InvaderPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        half_width = self._sprite.width / 2
        self.left = 64 + half_width
        self.right = 960 - half_width
        self.position = Vector2(self.left, u.INVADER_PLAYER_Y)
        self.free_to_fire = True
        self.fire_request_allowed = True
        self.shot_count = 0

class ReservePlayer(InvadersFlyer):
    def __init__(self, reserve_number):
        self.reserve_number = reserve_number
        maker = BitmapMaker.instance()
        players = maker.players  # one turret, two explosions
        self.player = players[0]
        self._rect = self.player.get_rect()
        half_width = self.rect.width / 2
        left = 64 + half_width
        x = left + reserve_number*(5*self._rect.width//4)
        self.rect.center = Vector2(x, u.RESERVE_PLAYER_Y)

class RobotPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        half_width = self._sprite.width / 2
        self.left = 64 + half_width
        self.right = 960 - half_width
        self.position = Vector2(self.left, u.INVADER_PLAYER_Y)
        self.count = 0
        self.invader_x_values = []
        self._can_shoot = True

This is immediately interesting, isn’t it? We notice:

  1. ReservePlayer is not Spritely, but makes its own bitmap. When we look, we see that it has its own draw method, while the other two do not, since Spritely handles that.
  2. The inits are very similar, with common terms like left and right and half_width and position turning up often.
  3. There seem to be similarities using different words, like free_to_fire and _can_shoot.
Note
Instead of removing the similarities by exploiting the differences in the classes, we’ll begin by making them more similar. That may seem like going in the wrong direction, but I think we’ll see that it is not: the more similar they are, the easier it will be to move similarity to common spots, isolating key differences while eliminating things that appear different but really are not.

I think what happens is this: when there are two pieces of absolutely identical code, we can easily find ways to make just one piece of code and two uses of that piece. So when we see “similar” code, if we can make it identical, we know we can do something good. That means that we can profit if we make similar code more similar.

Let’s proceed by making these objects more alike, at least here in __init__. First, let’s make ReservePlayer Spritely. That seems the most like actually programming, so I want to get it out of the way early. Or, I don’t know, maybe it’s the biggest difference, so I want to eliminate it. Or, I don’t know, I just want to start there.

If we get in trouble, relief is just a rollback away.

We’ll replicate the Spritely code, and remove the bitmap code and the draw method. That should just do it.

Almost. We also have to remove rect and mask properties, not shown, as they are provided by Spritely. The result is this:

class ReservePlayer(Spritely, InvadersFlyer):
    def __init__(self, reserve_number):
        self._sprite = Sprite.player()
        self.reserve_number = reserve_number
        half_width = self.rect.width / 2
        left = 64 + half_width
        x = left + reserve_number*(5*self.rect.width//4)
        self.rect.center = Vector2(x, u.RESERVE_PLAYER_Y)

We are green and the game works. We’ll commit this even though I see another thing to change. Commit: make ReservePlayer Spritely.

The last line there can be setting position, since Spritely provides that convenience.

class ReservePlayer(Spritely, InvadersFlyer):
    def __init__(self, reserve_number):
        self._sprite = Sprite.player()
        self.reserve_number = reserve_number
        half_width = self.rect.width / 2
        left = 64 + half_width
        x = left + reserve_number*(5*self.rect.width//4)
        self.position = Vector2(x, u.RESERVE_PLAYER_Y))

    def interact_with_destructor(self, destructor, fleets):
        fleets.remove(self)
        fleets.append(PlayerExplosion(self.position))

A quick search found that second use of self.rect.center and changed it as well. Commit: use position not rect center.

Now let’s review the inits again:

class InvaderPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        half_width = self._sprite.width / 2
        self.left = 64 + half_width
        self.right = 960 - half_width
        self.position = Vector2(self.left, u.INVADER_PLAYER_Y)
        self.free_to_fire = True
        self.fire_request_allowed = True
        self.shot_count = 0

class ReservePlayer(Spritely, InvadersFlyer):
    def __init__(self, reserve_number):
        self._sprite = Sprite.player()
        self.reserve_number = reserve_number
        half_width = self.rect.width / 2
        left = 64 + half_width
        x = left + reserve_number*(5*self.rect.width//4)
        self.position = Vector2(x, u.RESERVE_PLAYER_Y)

class RobotPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        half_width = self._sprite.width / 2
        self.left = 64 + half_width
        self.right = 960 - half_width
        self.position = Vector2(self.left, u.INVADER_PLAYER_Y)
        self.count = 0
        self.invader_x_values = []
        self._can_shoot = True

We want to make these look more alike. As a next step, we’ll put the different bits at the bottom, spaced one line apart.

class InvaderPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        half_width = self._sprite.width / 2
        self.left = 64 + half_width
        self.right = 960 - half_width
        self.position = Vector2(self.left, u.INVADER_PLAYER_Y)

        self.free_to_fire = True
        self.fire_request_allowed = True
        self.shot_count = 0

class ReservePlayer(Spritely, InvadersFlyer):
    def __init__(self, reserve_number):
        self._sprite = Sprite.player()
        half_width = self.rect.width / 2
        left = 64 + half_width
        x = left + reserve_number*(5*self.rect.width//4)
        self.position = Vector2(x, u.RESERVE_PLAYER_Y)

        self.reserve_number = reserve_number

class RobotPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        half_width = self._sprite.width / 2
        self.left = 64 + half_width
        self.right = 960 - half_width
        self.position = Vector2(self.left, u.INVADER_PLAYER_Y)

        self.count = 0
        self.invader_x_values = []
        self._can_shoot = True

What use are we making of the left and right members? In RobotPlayer, right is not used at all and left is only used right there in the setting of position. What about InvaderPlayer? It does use the values:

class InvaderPlayer(Spritely, InvadersFlyer):
    def move(self, amount):
        centerx = max(self.left, min(self._sprite.centerx + amount, self.right))
        self.position = Vector2(centerx, self.position.y)

    def x_fraction(self):
        x = self._sprite.centerx - self.left
        denom = self.right - self.left
        return x / denom

But those values are constants. Weird constants, but constants nonetheless. They do not vary by instance, nor by class.

Notice that self._sprite.centerx. Isn’t that self.position.x? Wouldn’t that be better? Yes. Especially there in move where we already use position.y. Fix those up. Commit: making things more alike.

If we can make those left and right things into constants, we can simplify these objects, making them more alike.

Let’s do a bit of refactoring in InvaderPlayer, starting here:

class InvaderPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        half_width = self._sprite.width / 2
        self.left = 64 + half_width
        self.right = 960 - half_width
        self.position = Vector2(self.left, u.INVADER_PLAYER_Y)

Let’s give Sprite a new method, half_width:

class Sprite:
    @property
    def half_width(self):
        return self.rectangle.width // 2

And use it:

class InvaderPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        self.left = 64 + Sprite.player().half_width
        self.right = 960 - Sprite.player().half_width
        self.position = Vector2(self.left, u.INVADER_PLAYER_Y)

        self.free_to_fire = True
        self.fire_request_allowed = True
        self.shot_count = 0

Now we can create constants in u.py:

u.py
INVADER_PLAYER_HALF_WIDTH = Sprite.player().half_width
INVADER_PLAYER_LEFT = 64 + INVADER_PLAYER_HALF_WIDTH
INVADER_PLAYER_RIGHT = 960 - INVADER_PLAYER_HALF_WIDTH

And plug them into InvaderPlayer:

class InvaderPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        self.position = Vector2(u.INVADER_PLAYER_LEFT, u.INVADER_PLAYER_Y)

        self.free_to_fire = True
        self.fire_request_allowed = True
        self.shot_count = 0

    def move(self, amount):
        centerx = max(u.INVADER_PLAYER_LEFT, min(self.position.x + amount, u.INVADER_PLAYER_LEFT))
        self.position = Vector2(centerx, self.position.y)

    def x_fraction(self):
        x = self.position.x - u.INVADER_PLAYER_LEFT
        denom = u.INVADER_PLAYER_RIGHT - u.INVADER_PLAYER_LEFT
        return x / denom

Three tests break:

class TestPlayer:
    def test_start_left(self):
        player = InvaderPlayer()
        assert player._sprite.centerx == player.left

    def test_left_edge(self):
        player = InvaderPlayer()
        player.move(-10000)
        assert player._sprite.centerx == player.left

    def test_right_edge(self):
        player = InvaderPlayer()
        player.move(10000)
        assert player._sprite.centerx == player.right

Fix those to reference the new constants. One still fails, finding the bug here:

    def move(self, amount):
        centerx = max(
            u.INVADER_PLAYER_LEFT, 
            min(self.position.x + amount, u.INVADER_PLAYER_RIGHT))
        self.position = Vector2(centerx, self.position.y)

I had LEFT instead of RIGHT. I had done those changes by hand, because I couldn’t think of a machine way to do it. My bad. Sorry.

We are green. We test in game. We are good. Commit: remove left and right members from InvaderFlyer, using u.constants.

Now we want the same changes in the RobotPlayer, which are trivial because it doesn’t use those members at all:

class RobotPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        self.position = Vector2(u.INVADER_PLAYER_LEFT, u.INVADER_PLAYER_Y)

        self.count = 0
        self.invader_x_values = []
        self._can_shoot = True

Now let’s use the constants in ReservePlayer:

class ReservePlayer(Spritely, InvadersFlyer):
    def __init__(self, reserve_number):
        self._sprite = Sprite.player()
        x = u.INVADER_PLAYER_LEFT + reserve_number*(5*self.rect.width//4)
        self.position = Vector2(x, u.RESERVE_PLAYER_Y)

        self.reserve_number = reserve_number

Let’s review the three inits again:

class InvaderPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        self.position = Vector2(u.INVADER_PLAYER_LEFT, u.INVADER_PLAYER_Y)

        self.free_to_fire = True
        self.fire_request_allowed = True
        self.shot_count = 0

class RobotPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.step = 4
        self.position = Vector2(u.INVADER_PLAYER_LEFT, u.INVADER_PLAYER_Y)

        self.count = 0
        self.invader_x_values = []
        self._can_shoot = True

class ReservePlayer(Spritely, InvadersFlyer):
    def __init__(self, reserve_number):
        self._sprite = Sprite.player()
        x = u.INVADER_PLAYER_LEFT + reserve_number*(5*self.rect.width//4)
        self.position = Vector2(x, u.RESERVE_PLAYER_Y)

        self.reserve_number = reserve_number

This is getting long, though it is mostly code, which I presume you mostly skim, but there’s a bit more we can do. Let’s promote that magic 4 to a constant:

u.py
INVADER_STEP = 4

class InvaderPlayer
    def check_motion(self, keys):
        if keys[pygame.K_f]:
            self.move(u.INVADER_STEP)
        elif keys[pygame.K_d]:
            self.move(-u.INVADER_STEP)

class RobotPlayer
    def one_step_toward_target(self, starting_x, target_x):
        if starting_x < target_x:
            return u.INVADER_STEP
        elif starting_x > target_x:
            return -u.INVADER_STEP
        else:
            return 0

Commit: magic numbers to constants. I got to going too fast here, and could have committed more frequently. Bad habit, but no real harm done … this time. As far as we know …

One more look at the three:

class InvaderPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.position = Vector2(u.INVADER_PLAYER_LEFT, u.INVADER_PLAYER_Y)

        self.free_to_fire = True
        self.fire_request_allowed = True
        self.shot_count = 0

class RobotPlayer(Spritely, InvadersFlyer):
    def __init__(self):
        self._sprite = Sprite.player()
        self.position = Vector2(u.INVADER_PLAYER_LEFT, u.INVADER_PLAYER_Y)

        self.count = 0
        self.invader_x_values = []
        self._can_shoot = True

class ReservePlayer(Spritely, InvadersFlyer):
    def __init__(self, reserve_number):
        self._sprite = Sprite.player()
        x = u.INVADER_PLAYER_LEFT + reserve_number*(5*self.rect.width//4)
        self.position = Vector2(x, u.RESERVE_PLAYER_Y)

        self.reserve_number = reserve_number

Let’s reflect and sum up.

Reflecto-Summary

These objects are much more alike than they were, and they are all somewhat simpler, referring to manifest constants rather than making calculations, and with fewer member variables and more common access to get information that they need. Our changes have had small impact on the methods inside the classes, mostly replacing member references with constant references. Our tests have found one actual mistake and have served as checks on our changes throughout.

We can see some likely candidates for making them more similar, such as count and shot_count, which may or may not be similar, and free_to_fire, fire_request_allowed, and _can_shoot, which almost certainly will lead to code that is similar in some ways.

What has happened here? We have observed a few times that these three classes have a lot of similarity and we are interested in exploiting that similarity to make the code better, presumably via subclassing, delegation, and the like. Instead of exploiting the similarities, we have mostly made even more similarity.

But we have in fact exploited some of those similarities in two important ways.

Most obviously, we have extracted code and converted it to constants, which keeps the similarities similar. Prior to those changes we could have mistakenly broken those key left and right relationships: now we cannot. We removed duplicate code to constants. If that isn’t a named refactoring somewhere, it should be.

Less obviously, we removed some duplication entirely. Move Code to Oblivion, that one might be called.

What remains still has similarities … which we will probably try to make more similar … and differences, which we will explore soon.

My feelings during this session are interesting. I’ve been quite relaxed, never a bit of tension or worry. I’ve been feeling mildly pleased the whole time, as the code-pushing and pulling made the objects more and more alike. No big spikes of geek joy, but a slow suffusion of a low level of the thing that keeps me going, the sheer pleasure of programming going well.

This is what I like, and what my practices, as I thoughtfully adjust them, are meant to provide: pleasure in doing the work. Geek joy, we call it.

See you next time!