It’s time for more features and less playing with drawing. This leads me to think about common behavior in motion and collisions. Ron learns something about inheritance and delegation!

All the objects in Asteroids move similarly. They have a velocity and on every clock tick, they adjust their position by adding the velocity to their position. The amount added is scaled to the length of the clock tick, to keep their motion consistent.

Similarly, object collisions are all quite similar. We compare two objects to see if their distance apart is small enough that they are colliding, and if they are, each object does whatever it does upon colliding. Ships and saucers explode, missiles just expire, asteroids split or vanish depending on size. Again, quite similar.

Now this makes me think of inheritance. If certain bits of behavior are the same, one way of dealing with that is to put the behavior in a superclass, and subclass all the objects from that class. But there are issues.

First, as Kent Beck once put it: “Subclassing is a card you can only play once. It is best to keep it in your hand as long as possible.”

Second, there are those who hold that inheriting concrete behavior is problematical. My friend GeePaw Hill put it this way yesterday:

Without wanting to write a full-length essay. Implementation inheritance is anti-change. It makes changing code harder, and I am in the business of changing code.

It multiplies classes, which is fine, but it also intertwingles them in change-resistant ways. It chops holes into those classes’ boundaries, which is not fine. The more holes such a boundary has in it, the more difficult it is to grasp its actual meaning.

It hardens designs prematurely. You can’t substantively change a superclass without instantly substantively changing all of its subclasses. This forces one to be right and to be right fast, and that is a Bad Thing™.

If your super-class has concretes and abstracts mixed, then it is untestable without a subclass instantiation. Things that are purpose-built to be untestable are also a Bad Thing™.

The question for me isn’t why is II bad, it’s why is II good? The two answers you’ll see over and over again are bogus, in my view. Answer 1) I avoid the typing I have to do in language X to use delegation instead of II. Answer 2) It perfectly captures the abstraction in the finished product.

The first is bogus cuz typing isn’t what’s hard about programming.

The second is bogus cuz my job is changing code, and finish-lines are rare, and II does more damage when it’s wrong than good when it’s right.

I think you (Ron) don’t see it for three reasons: a) because you’re highly skilled at composition and factoring. 2) you do small projects these days. and iii) you work solo.

None of a, 2, or iii are the norm in our trade.

The bottom-line: implementation inheritance is a technique whose primary value is increasing complexity while reducing typing, and typing is not the bottleneck.

Hill goes on to say:

One of the challenges with objecting to Implementation Inheritance is that the problems it creates tend to develop insensibly. Everything goes well for a while, then splat you got [DELETED] all over you. Issues like that are very hard to explain, because the concrete examples require so much code to show.

I (Ron) think this is a fair cop, or nearly so. We could quibble about “always and never are always an exaggeration and never quite accurate”. We could show examples that are easy to understand, “proving” that inheriting implementation is sometimes the best thing, and so on. But I have to agree that my willingness to use it is in part due to reasons a, 2 and iii above, the fact that I work alone, on small things, and (possibly) I haz some skillz.

Fortunately …

There are alternatives to subclassing. The most common is delegation. We could have all of asteroid, missile, saucer, and ship have a mover that is called when we want to move them. We could do that in at least two ways. In both cases the objects would have a mover. We could either have the objects all implement move and say self.mover.move, or we could have the existence of the mover be known by the objects’ owner and it would say object.mover.move. The first way would allow us to have a mover or not as a given object might prefer, and the second requires that all the objects provide a mover. Of course, an object might provide itself. I think it comes down to a matter of taste.

When we use the delegation approach, we often refer to it as “composition”, because our objects are composed of other objects, movers, colliders, and so on.

I think it’s fair to say that a design based on composition is usually thought to be better than one based on concrete inheritance.

But what about Asteroids?

Our problem is very simple and we can surely do an adequate job either way. Although when I started this discussion I was thinking I’d use inheritance, I’ve convinced myself that composition is probably better and in any case worth a try. So what say we start now, moving toward a design where our objects have at least a separate mover and quite likely a separate collider when we get there. Maybe even a drawer (draw-er, not the thing with your socks in it).

This seems a bit like Entity Component System?

Indeed it does. We’ll certainly have entities and components, but probably not systems. So it’s similar. But then, everything is similar if you hold your head just right.

Let’s get Started

New rule: all the objects have a mover and the main loop calls it to move them. Moving, this morning, is done in the main loop, by this code:

    ship.move(dt)
    for asteroid in asteroids:
        asteroid.move(dt)

What I’d like is for the mover to be a property. So, let’s just work by intention here and code this:

    ship.mover.move(dt)

As soon as I type that, PyCharm offers to add a field to Ship. Very clever, these JetBrains people. I’ll let it do it.

class Ship:
    def __init__(self, position):
        self.mover = None
        self.position = position
        self.velocity = Vector2(0, 0)
        self.angle = 0
        self.acceleration = u.SHIP_ACCELERATION
        self.accelerating = False
        ship_scale = 4
        ship_size = Vector2(14, 8)*ship_scale
        self.ship_surface, self.ship_accelerating_surface = SurfaceMaker.ship_surfaces(ship_size)

Good guess, but in fact I prefer:

class Ship:
    def __init__(self, position):
        self.mover = self

It’s clear on the face of it that that will work. So, naturally, I test it. And it does. Commit: Ship now provides a mover (self).

I think we have a plan. Now we’ll do the asteroids the same way.

    ship.mover.move(dt)
    for asteroid in asteroids:
        asteroid.mover.move(dt)

class Asteroid:
    def __init__(self, size=2, position=None):
        self.mover = self

That works, unsurprisingly. Commit: Asteroid now provides a mover (self).

Now we are here to create a mover, not just to pretend that we have one. Let’s examine the two objects’ existing move code.

class Ship ...
    def move(self, dt):
        self.position += self.velocity * dt
        self.position.x = self.position.x % u.SCREEN_SIZE
        self.position.y = self.position.y % u.SCREEN_SIZE

class Asteroid ...
    def move(self, dt):
        self.position += self.velocity * dt
        self.position.x = self.position.x % u.SCREEN_SIZE
        self.position.y = self.position.y % u.SCREEN_SIZE

There’s a certain similarity there. We can readily create a little object with that code, but there are questions in my mind: should that object own the position and should it own the velocity?

If it owns position, then drawing will require access to the mover. For asteroid, here’s draw:

    def draw(self, screen):
        top_left_corner = self.position - self.offset
        pygame.draw.circle(screen, "red", self.position, 3)
        screen.blit(self.surface, top_left_corner)

We could change those to say self.mover.position and it would work. Or we could implement a property, if we knew how to do that. Let’s do it with the reference through mover, which should just work because we are mover. We’ll see about unwinding it shortly.

    def draw(self, screen):
        top_left_corner = self.mover.position - self.offset
        pygame.draw.circle(screen, "red", self.mover.position, 3)
        screen.blit(self.surface, top_left_corner)

Should “just work”. Does. Let’s look at all references to position and do that.

No. Gets too recursive. Back that out. Roll back. Let’s build an install a mover. We’ll TDD the mover itself, should be trivial.

I don’t think this will quite hold water but it expresses the idea:

    def test_mover(self):
        pos = (100, 200)
        vel = (10, 20)
        mover = Mover(pos, vel)
        mover.move(0.5)
        assert mover.pos == (105, 210)

And

class Mover:
    def __init__(self, position, velocity):
        self.position = position
        self.velocity = velocity
        
    def move(self, deltaTime):
        self.position += self.velocity*deltaTime
        self.position.x = self.position.x % u.SCREEN_SIZE
        self.position.y = self.position.y % u.SCREEN_SIZE

I’m not sure whether I can add to those sequences. They should be Vector2 anyway, but let’s see what Python says.

    def move(self, deltaTime):
>       self.position += self.velocity*deltaTime
E       TypeError: can't multiply sequence by non-int of type 'float'

Right. Improve the test:

    def test_mover(self):
        pos = Vector2(100, 200)
        vel = Vector2(10, 20)
        mover = Mover(pos, vel)
        mover.move(0.5)
        assert mover.position == Vector2(105, 210)

That passes. I built the class inside the test file. Move it to its own little file.

Now let’s use it in Asteroid. Starting from this:

class Asteroid:
    def __init__(self, size=2, position=None):
        self.mover = self
        asteroid_sizes = [32, 64, 128]
        try:
            asteroid_size = asteroid_sizes[size]
        except IndexError:
            asteroid_size = asteroid_sizes[2]
        self.offset = Vector2(asteroid_size/2, asteroid_size/2)
        self.position = position if position else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
        angle_of_travel = random.randint(0, 360)
        self.velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
        self.surface = SurfaceMaker.asteroid_surface(asteroid_size)

We’ll do this:

class Asteroid:
    def __init__(self, size=2, position=None):
        asteroid_sizes = [32, 64, 128]
        try:
            asteroid_size = asteroid_sizes[size]
        except IndexError:
            asteroid_size = asteroid_sizes[2]
        self.offset = Vector2(asteroid_size/2, asteroid_size/2)
        mover_position = position if position else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
        angle_of_travel = random.randint(0, 360)
        mover_velocity = velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
        self.mover = Mover(mover_position, mover_velocity)
        self.surface = SurfaceMaker.asteroid_surface(asteroid_size)

Now we have PyCharm concerned by this code:

    def draw(self, screen):
        top_left_corner = self.position - self.offset
        pygame.draw.circle(screen, "red", self.position, 3)
        screen.blit(self.surface, top_left_corner)

The easy fix is just to ask the mover. Let’s do that. And I think we ought to be able to remove the move method entirely. PyCharm thinks there are users. I think it’s mistaken. Remove the method. Tests pass, asteroids continue to fly and be drawn. Nice.

I think I have time to do the ship before breakfast.

    def __init__(self, position):
        self.mover = self
        self.position = position
        self.velocity = Vector2(0, 0)
        self.angle = 0
        self.acceleration = u.SHIP_ACCELERATION
        self.accelerating = False
        ship_scale = 4
        ship_size = Vector2(14, 8)*ship_scale
        self.ship_surface, self.ship_accelerating_surface = SurfaceMaker.ship_surfaces(ship_size)

That looks easy enough. However, the game explodes when I try to accelerate the ship. I forgot to change this:

    def power_on(self, dt):
        self.accelerating = True
        accel = dt * self.acceleration.rotate(-self.angle)
        self.mover.velocity += accel

With that we’re good in the game. However, my auto-test running must not be working because we have tests failing all over. I fix things by inserting .mover a lot.

It’s about time for me to break for breakfast, so let’s sum up.

Summary

The good news is that we’ve devised a Mover delegate and are using it in both ship and asteroid. I might even argue that it is more clear than inheriting move from a superclass would be. In the inheritance mode, we’d see that ship can move and when we went to look to see how it worked, we wouldn’t find it. After looking around a while, we would find it in the superclass. No big deal.

But with delegation, as done here, the code says explicitly ship.mover.move(dt), so we know instantly that ship has a mover and the mover moves. The knowledge comes to us immediately. And had we done it the other way, preserving the move method on ship, it would be implemented as self.mover.move(dt) and again we understand instantly what is happening.

One more nail …

In the brief conversation with Hill, I said this:

An offering: this morning I implemented Mover, a thing with position and velocity, that updates position at intervals. Delegating moving to Mover removes the duplicated move methods in, well, everyone who moves. One could of course just move the move method to a superclass. However, doing that is, or might probably be, just blind text manipulation without any real attention to the idea. The “solution” of moving it to the superclass risks mindless changes. Devising the delegate may take a little longer, but better captures the idea behind the change.

I may have convinced myself. Fascinating. I’m glad I asked Hill to do his little writeup.

A small issue with the delegation was that we had to change about four or five tests to say .mover, because they wanted to get or set the ship’s velocity or position. We could have avoided that by doing the forwarding inside the class, of course.

And I still don’t know how to do a property in Python, but I’ll look it up later this morning. I don’t really want to use one just now. We could implement, say, velocity on ship to forward to velocity in the mover, but I’d rather change the tests, at least for now.

Anyway we’re good and green, so commit: ship and asteroid both use Mover.

See you next time!