Python 96 - Shhhh

I am going to do a thing, and then another thing. Don’t tell Hill about the other thing.

When implementing a new flyer object, I’ve been bitten a few times by failing to implement one or another of the required methods in a flyer. We have a test that checks them, but I have to remember to add new required methods, and to add the new class to the list of classes to be checked:

    def test_flyer_protocol(self):
        classes = [Asteroid, Missile, Saucer, Ship, Fragment, Score, ScoreKeeper]
        errors = []
        for klass in classes:
            attrs = dir(klass)
            methods = ["interact_with",
                       "interact_with_asteroid",
                       "interact_with_missile",
                       "interact_with_saucer",
                       "interact_with_ship",
                       "interact_with_score",
                       "interact_with_scorekeeper",
                       "are_we_colliding",
                       "tick",
                       "draw"]
            for method in methods:
                if method not in attrs:
                    errors.append((klass.__name__, method))
        assert not errors

I’m going to do a thing, and then another thing:

The Thing: I’m going to build an abstract superclass for the flyers. In it, I will create abstract methods for the methods that must be implemented. After that’s done, I know that Python will refuse to instantiate any subclass that doesn’t implement all the abstract methods, and I suspect that PyCharm will check them even before that.; All the flyers will inherit this new superclass, which will amount to a defined interface that they must implement.

I don’t recall whether I wrote up removing the superclass that I used to have. One Tuesday evening, trying to figure out PyCharm’s refactoring, the Friday Geek’s Night Out Zoom Ensemble put a superclass in, to see whether PyCharm would refactor methods if it knew they were in a subclass of the class you were changing. Turns out that it does, at least sometimes.

I later decided that the superclass wasn’t helping me, and I removed it. I don’t recall whether I mentioned it in an article or not. Anyway, if you’re wondering where it went, that’s what happened.

Now we’re going to put it back, only better.

Let me show you what I’m putting in and then explain it a bit.

from abc import ABC, abstractmethod


class Flyer(ABC):

    @abstractmethod
    def interact_with(self, other, fleets):
        pass

    @abstractmethod
    def tick(self, delta_time, fleet, fleets):
        pass

The abc import refers to Python’s standard abstract class module, and we import a reference to the class ABC, the standard base class to which you refer when building an abstract base class. We also refer to abstractmethod, the decorator that you use to define a method as abstract, which is to say, a method that is required to be implemented by all subclasses of this abstract class.

You may be wondering about the other required methods. Patience, Prudence, we’ll get there.

I want all my flyer classes, all the space objects, to inherit from Flyer. I’ll try to remember them all and one good clue will be the list in the test above, which is current as of yesterday.

class Asteroid(Flyer):
class Fragment(Flyer):
class Missile(Flyer):
class Saucer(Flyer):
class Score(Flyer):
class ScoreKeeper(Flyer):
class Ship(Flyer):

I did that “from memory” and forgot Fragment. Then I checked the list and found it.

Is there a way to determine whether a class is a subclass of another? There sure is. Let’s add it to the test:

    def test_flyer_protocol(self):
        classes = [Asteroid, Missile, Saucer, Ship, Fragment, Score, ScoreKeeper]
        errors = []
        for klass in classes:
            assert issubclass(klass, Flyer)
            attrs = dir(klass)
            methods = ["interact_with",
                       "interact_with_asteroid",
                       "interact_with_missile",
                       "interact_with_saucer",
                       "interact_with_ship",
                       "interact_with_score",
                       "interact_with_scorekeeper",
                       "are_we_colliding",
                       "tick",
                       "draw"]
            for method in methods:
                if method not in attrs:
                    errors.append((klass.__name__, method))
        assert not errors

Test passes. Brilliant.

Commit: all flyers subclass abstract class Flyer.

Now I want to experiment to see what happens if I forget to implement one of those methods. Let’s randomly pick ScoreKeeper, because it’s in my tabs at the moment.

When I comment out the interact_with, PyCharm under-squiggles the class name and there is a new warning “Class ScoreKeeper must implement all abstract methods”. I’d prefer a stronger message but Python allows for the possibility that you might implement an abstract method at run time. There’s even an official way to do it and register it. I do not advise learning that.

Let’s reflect.

Reflection

We have an abstract superclass defined for all our flyers. Any method that we put in there, marked @abstractmethod, will be required in all subclasses. So far, I’ve only marked tick and interact_with. Our test for proper protocol checks a lot of methods:

    methods = ["interact_with",
               "interact_with_asteroid",
               "interact_with_missile",
               "interact_with_saucer",
               "interact_with_ship",
               "interact_with_score",
               "interact_with_scorekeeper",
               "are_we_colliding",
               "tick",
               "draw"]

Shouldn’t we mark all these methods as @abstractmethod, so that PyCharm will help us out, and so that we’ll be even more safe than we are with this test in place.

I am going to say, “no, there is a preferable way”. I say “preferable”, not “better”, because not everyone will agree that it is better but no one can disagree that I prefer it.

The Other Thing: I propose to make a number of methods in Flyer concrete, i.e. not marked abstract, and to give them default implementations. And then, for all the subclasses whose desired implementation of those methods matches the default, I’m going to remove the specific implementation, allowing the class to inherit the default.; You must not tell Hill about this. He will freak, or, worse, frown at me for violating a famous rule of object-oriented software that advises against inheriting concrete behavior.; Now let me say right here, that I know this rule, even if it’s more of a guideline than an actual rule, and I am inclined to adhere to it, and if I were to give recommendations, which I generally do not, I would recommend that people not inherit concrete implementation. The practice nails your foot to the floor, seriously reducing the flexibility you have with regard to future subclassing. Probably. Maybe. Sometimes.; All that said, I’m going to do it, I’m going to try to do it thoughtfully, and I’m going to pay attention to what happens after I do it, observing whether I get in trouble or not. If it doesn’t work out, it’ll be easy to change it to a more strict form.
Current abstract methods: Let’s consider the abstract methods we have so far, just are_we_colliding and tick.; I think that every object should explicitly deal with tick, I’m not sure why I think that, but I do. What about are_we_colliding? Let’s review implementations.; Fragment returns False. Fragments collide with no one. Score returns False., as does ScoreKeeper. Asteroid, Missile, Saucer, and Ship implement an identical distance check.
Two Distinct Sub-groups: There seem to be two distinct sub-groups here, don’t there? There are colliding flyers, and non-colliding flyers. Should we make a little hierarchy here? If we did, we could push the are_we_colliding code up. Hill would frown even more if we were to do that … but we could. We really could.; Given that separation, and the fact that I could so easily name it, we might profit from considering two different interfaces, one for colliders and one for non-colliders. But for now, there’s no clear default for the are_we_colliding and we won’t provide one. My eyes are more on the specialized methods interact_with_asteroid and so on.
interact_with: But first, what about interact_with? Should that be required? Here, each class must implement this as interact_with_klass, where klass is its own class name. There is an exception: Fragments interact with no one, and they implement the method as pass but pass is surely not a good default. So interact_with should be abstract in Flyer.
interact_with_: What about the various class-specific interact_with_ methods? Those, I claim, could be defaulted to pass and then the rule would be that each object would only define the methods it wants to deal with. Take Asteroid for example, which says this:

class Asteroid:
    def interact_with_asteroid(self, asteroid, fleets):
        pass

    def interact_with_missile(self, missile, fleets):
        if missile.are_we_colliding(self.position, self.radius):
            fleets.add_score(Score(self.score_for_hitting(missile)))
            self.split_or_die(fleets)

    def interact_with_saucer(self, saucer, fleets):
        if saucer.are_we_colliding(self.position, self.radius):
            self.split_or_die(fleets)

    def interact_with_score(self, score, fleets):
        pass

    def interact_with_scorekeeper(self, scorekeeper, fleets):
        pass

    def interact_with_ship(self, ship, fleets):
        if ship.are_we_colliding(self.position, self.radius):
            self.split_or_die(fleets)

This most collision-oriented of objects still calls a pass on fully half the possibilities, and there are more to come, such as ShipChecker, that it will want not to deal with. Even with WaveChecker, which will probably count asteroids, we probably do not need the asteroids to deal with it. (There is a choice to be made there, but I’m speculating.)

So I think we should implement the interact_with_ methods all in Flyer, set them all to pass, and then not make them abstract. Then we can remove the noise from the individual subclasses.

There are other ways to accomplish something similar. For example, we could rig an explicit or simulated event structure, and trigger interact_with_ events, and other objects could subscribe or not, depending on their level of interest. We had a very clever Hill-created scheme in the Kotlin decentralized version.

Honestly, I think inheritance is simpler, and just as explicit. I’m going to do it, so there.

We’ll implement all the interact_with_ methods in flyer, as concrete pass.

class ScoreKeeper(Flyer):
    def __init__(self):
        self.score = 0

    def are_we_colliding(self, position, radius):
        return False

    def draw(self, screen):
        pass

    def interact_with(self, other, fleets):
        other.interact_with_scorekeeper(self, fleets)

    def interact_with_asteroid(self, asteroid, fleets):
        pass

    def interact_with_missile(self, missile, fleets):
        pass

    def interact_with_saucer(self, saucer, fleets):
        pass

    def interact_with_score(self, score, fleets):
        self.score += score.score

    def interact_with_scorekeeper(self, scorekeeper, fleets):
        pass

    def interact_with_ship(self, ship, fleets):
        pass

    def tick(self, delta_time, _fleet, _fleets):
        pass

Green. Commit: implement default interact_with_ methods.

Now, carefully, we’ll remove the pass methods from other objects. Let’s do the big winners first, Score and ScoreKeeper. Here’s ScoreKeeper:

class ScoreKeeper(Flyer):
    def interact_with_asteroid(self, asteroid, fleets):
        pass

    def interact_with_missile(self, missile, fleets):
        pass

    def interact_with_saucer(self, saucer, fleets):
        pass

    def interact_with_score(self, score, fleets):
        self.score += score.score

    def interact_with_scorekeeper(self, scorekeeper, fleets):
        pass

    def interact_with_ship(self, ship, fleets):
        pass

I can just remove all the pass ones. The resulting class:

class ScoreKeeper(Flyer):
    def __init__(self):
        self.score = 0

    def are_we_colliding(self, position, radius):
        return False

    def draw(self, screen):
        pass

    def interact_with(self, other, fleets):
        other.interact_with_scorekeeper(self, fleets)

    def interact_with_score(self, score, fleets):
        self.score += score.score

    def tick(self, delta_time, _fleet, _fleets):
        pass

This seems rather neat, clean, and clear to me. I note the implementation of draw. I think we should require that method, so I’ll add an abstract one in flyer. But first, let’s commit this: remove default interact_with methods.

Now add draw to Flyer.

class Flyer(ABC):
    @abstractmethod
    def draw(self, screen):
        pass

Commit: draw now abstract in Flyer, i.e. required in subclasses.

Let’s simplify the Score, removing all the methods it can now inherit, removing:

class Score(Flyer):

    def interact_with_asteroid(self, asteroid, fleets):
        pass

    def interact_with_missile(self, missile, fleets):
        pass

    def interact_with_saucer(self, saucer, fleets):
        pass

    def interact_with_score(self, score, fleets):
        pass

    def interact_with_ship(self, ship, fleets):
        pass

The class is now just this:

class Score(Flyer):
    def __init__(self, score):
        self.score = score

    def are_we_colliding(self, position, radius):
        return False

    def draw(self, screen):
        pass

    def interact_with(self, other, fleets):
        other.interact_with_score(self, fleets)

    def interact_with_scorekeeper(self, scorekeeper, fleets):
        fleets.remove_score(self)

    def tick(self, delta_time, _fleet, _fleets):
        pass

I am liking this. It’s direct, and simpler than it would be to add in a subscription. I think we have other matters before the committee this coming Tuesday but sooner or later I’m sure we’ll have a deep discussion on this. Or not.

Let’s do the other classes. I’ll spare you the details.

Fragment commit: remove defaulted interact_with_ methods Asteroid; Missile; Saucer; Ship. All ditto.

Let’s reflect, and perhaps then just sum up.

Reflection

We have taken the questionable step of implementing concrete methods in our not quite fully abstract class Flyer, providing default (but empty) methods for each of the interact_with_ methods, of which there are now six.

Notably, our test that checks implementations still runs green, which means that the dir function must return all the methods of a class plus all those in its superclasses. A quick read of the Python spec affirms this. Good to know.

At the most superficial level of thinking, I prefer this scheme because each class’s visible interface is narrowed. We only see the methods that it explicitly needs. The default methods approach has removed a minimum of two methods from each class, to a maximum of, what, five? And there will be more that we won’t have to deal with.

Each time we add a new interact_with_mumble method, we would have had to add that method, typically with a pass, to every class of Flyer. Now we don’t have to do that. I think that’s a clear win.

A closer look …

However, drilling down a bit, what mistakes might future Ron, or some other programmer, make?

If we were to implement a new class, Foo, with which several existing classes needed to interact, we would have to put an interact_with_foo into all those classes. If we didn’t make interact_with_foo abstract, we might forget, and a missed object would not work correctly. Is that a possible thing? Yes, it is. Let’s think of examples.

Suppose we wanted to add a special weapon to the ship, perhaps one that can only be used once, which draws an expanding wave and wipes out everything it touches. We’d implement that by dropping a BigKiller object into the mix and every other colliding flyer would implement interact_with_killer and remove itself with suitable fireworks. If we forgot to implement that method on some class, that class would be impervious to the BigKiller and that would not be our intention.

All we have to do to avoid that is to implement interact_with_killer as abstract in Flyer, should we care to. It would be even easier if we were to implement two new subclasses of Flyer, CollidingFlyer and NonCollidingFlyer. We should perhaps do that.

Even closer …

Digging even deeper into the concerns bag, we might worry that some future implementation of something will be made more difficult because of these defaults. It’s hard to prove a negative but I don’t see such a thing, and since all our concrete defaults are just pass, there’s nothing likely that can’t be dealt with by overriding the default, which is not only legal, it is the expected thing if you want behavior.

I am giving this implementation my blessing. We’ll see what my peers have to say.

Nota Bene: You should note well, thus the heading just above, that doing this simple change has given us a lot to think about in a somewhat more complicated way than we might have thought about it before.; Before, we would have said “if there’s an interact_with_foo, every flyer has to implement it”. Now, we have options. We can make it abstract and then everyone will have to implement it, or concrete, in which case only interested parties have to implement it.; There was only one kind of mistake before: forget to implement it. Now there are more kinds: forget to add it to Flyer at all, add it as concrete and forget to implement it somewhere it’s needed, add it as abstract and pay the price of lots of places where it has to be implemented.; This scheme is a bit more intricate than what went before. We have options and while they are valuable, they also add a bit of complexity. So it goes.

Summary

I think by any standard, these changes went smoothly, in a series of nine commits over a period of less than 90 minutes from the first to the last. We have added a powerful feature to the code, in an abstract class that will enforce the implementation of methods we deem to be required, such as tick and draw, and interact_with.

This will give us a warning if we forget those methods in any future Flyer class, and I think PyCharm will even offer to implement shell methods when we create new classes.

Python will also refuse to instantiate an object that doesn’t implement all the abstract methods, and I consider that to be a potential problem. So long as we create at least one instance in our tests, we’ll be OK, but if there are no tests for an object, it is possible that it could cause a failure at run time, and it’s at least credible that such a thing might slip through game-play testing.

Are there any classes with zero tests? I don’t think so, but I wouldn’t bet my cat on it.

We’ll keep an eye on how things go. I think I prefer this scheme to just implementing things without an interface … and I think I prefer allowing both abstract, required methods, and concrete defaulted ones.

We’ll see how things go.

See you next time!