P-255 - Eternal Struggle

Python Asteroids+Invaders on GitHub

The Invaders team has encountered a troubling issue, and have been making some troubling decisions. We need to explore this and deal with it at its root. Spoiler: It’s not the programmers. People over process.

Summary (TL/DR)

If there is a big lesson here, it is that when the rules say one thing and the programmers are doing another thing, the problem is more likely to be with the system than with the programmers. This is true in code, in architecture, and in day to day work. People are more important than process—I forget who said that—and we should fix the process, not the people.

Issue (yet again)

The Invaders team (me) wanted a quick implementation of GameOver, and tried tossing a GameOver instance into their mix. It didn’t work. The error was not really surprising: GameOver does not implement methods like interact_with_bumper from the Invaders side of the house. They sat down with the Asteroids team (me) to talk about the issue and what should be done about it.

During the “discussion”, the Invaders team (me) stated that they have been breaking the rules, by implementing new classes with their interact_with method defined as pass. The implementation convention is that it should always be done like thisL

class NewThing(InvadersFlyer):
    def interact_with(other, fleets):
        other.interact_with_newthing(self, fleets)

Their reason for wanting to default to pass is simple: if NewThing implements interact_with as it is supposed to, then all the other objects have to be updated to respond to interact_with_newthing, which is a pain, takes precious time, is quite boring, and generates large useless commits.

A productive discussion ensued, because the two teams (me, and on the other team, also me) get along almost surprisingly well. The Asteroids team allowed as how they, too, had struggled with this issue. The Asteroids team had developed a different way of dealing with the issue. All their objects do implement interact_with according to the convention, but they default a large number of interact_with_specific_class methods in their interface.

They popped up their code on the screen and, sure enough, they had no fewer than nine defaulted interact_with_x methods in their interface, while, so far, the Invaders team had none. The Invaders team had three classes implementing interact_with as pass: PlayerExplosion, PlayerMaker, and TimeCapsule.

Kind of an interesting result.

We have one problem: it’s a pain to have to implement interact_with_newthing in all your classes every time a NewThing comes along.

We have two solutions: 1) give everyone a default interact_with_newthing, or 2) implement NewThing.interact_with as pass so that there is no method interact_with_newthing and no one is hassled.

We’ve talked about this before, extensively, and as yet we’ve found no satisfying way to balance the forces that we sense:

Avoid Implementation Inheritance: Inheriting concrete implementation is frowned upon by a lot of very intelligent and thoughtful people. Curiously, in our teams’ cases, each team had decided to use implementation inheritance, against advice, and they used it differently, one team at the higher level, one lower down in the calling sequences.; The two teams (me, and me) could not make a very good case against implementation inheritance. They had the received wisdom that it is thought by many to be bad, but had no solid reasons why. The best they could come up with is that at least abstract methods give you a moment where you really should think, for each class, “how does this class interact with NewThing”, and answer the question explicitly. They also recognized that both teams (me) had often just accepted the PyCharm pass, or thought “it doesn’t” and typed pass themselves.
The reminder didn’t really work like theory said it would. In flow, one just thinks “nope”, types pass, and moves on.: The teams believe that the opportunity to think isn’t paying off, but there may be better reasons to avoid implementation inheritance that they’re not aware of. Even in their own code, they can see that those defaulted pass overrides are often surprising when you expect an interaction and don’t get one. That’s particularly true when the Invaders team decides that an object doesn’t interact at all. That takes the decision out of the hands of all the other objects in the system.
Small Simple Changes: If we follow the strict rules of always implementing interact_with and interact_with_newthing explicitly, we are forced into large commits every time we simply add a new object. Almost invariably only a few existing classes really do want to interact with NewThing. The teams agree that the average number of existing classes that need to interact with a NewThing is probably less than two. But the work to be done grows with each new class.; Following the current rule slows us down, so we break it, each in our own way.

At this point, the teams come to an interesting conclusion. While they are concerned, in principle, that using implementation inheritance may well be bad, they observe that each team has independently (me, as opposed to on the other hand, me) decided to use inheritance as judiciously as they can, in aid of faster progress and smaller steps.

At the next level of detail, the teams believe that default individual class-focused interact_with_newthing methods to pass is a better idea than implementing interact_with as pass, because the latter means that no one can ever interact with that object until it is changed to conform to the conventions.

Desire Paths

: There’s the notion of “Desire Paths” in urban and campus planning. The idea is that you don’t put in sidewalks (pavements) right away: you wait to see where people choose to walk, and you put the pavement there. Supposedly a number of university campus paved paths were built this way. If people walk on the grass, it’s because that path is more desirable than the ones that exist.: The interesting thing about this idea is that it turns the mind around. Walking on the grass is no longer wrong: The grass is in the wrong place. We improve the pavement system rather than penalize people for walking on the grass.

We’re dealing with a Desire Path here. Our teams want their objects to default to ignoring interactions where they would otherwise explicitly pass, so that each object only implements the interactions that it actually uses. They agree that there is a risk that a new object will be created, and some existing object should interact with it, but that no one will remember and there will be a hole in the system’s logic. Buried in everyone’s memory is a vague recollection that that actually happened at some point, introducing a defect. No one (me) remembers the details.

What about delegation?

In my Kotlin version of Asteroids, my brother Hill devised a delegation scheme and we paired on implementing it in my program. It served nicely.

The essential idea was that each Flyer must implement a new method, interactions, which returned an instance of a new class, Interactions. When you create your class’s specific interactions method, you include pointers to all the interaction functions you care to support.

    override val interactions: Interactions = Interactions(
        interactWithScore = { score, trans ->
            totalScore += score.score
            trans.remove(score)
        }
    )

The Interactions class defaults to ignore any interactions that are not provided.

This was very neat, and avoided the use of implementation inheritance … in essence by implementing default do-nothing behavior in Interactions instead of inheriting it from the interface / superclass. It was a very nifty bit of programming, showing how one might use delegation instead of inheritance. Is it worth doing something like that in our games?

The teams are doubtful, but there was some interest in trying a spike or two, to see what it might be like in Python. That card has been tabled for now but someone (me) might try it at some future time.

Meanwhile, the working decision is that it is OK to default specific interact_with_newthing methods but that it is unsafe to default the basic interact_with, and that the Invaders team will back that change out as soon as they reasonably can.

But the original question …

We’re left, though, with the question of using one game’s class in another game. The teams came up with a really good example that hasn’t come up yet: Score. In Asteroids, when points are scored, a Score instance is tossed into the mix and is swept up by the ScoreKeeper, who reads out the score info and adds points to whichever player is active. It’s clear that Invaders would benefit from using the same scheme, and the same object.

Now the Score class is only 35 lines long, and half of those lines are definitions of methods with pass. It only has enough real code to store a score and to remove itself when it sees a ScoreKeeper. It would be trivially easy for the Invaders team to implement their own. And they already discovered that they can’t just reuse GameOver, although since coin.py is available to anyone, they could just issue a coin.slug to get back to the main game over / startup state.

We built AsteroidFlyer and InvadersFlyer interface / superclasses to make it easy for the two games to coexist. With that setup, neither game needs to know anything about the classes specific to the other game, and that seemed to be of value. The primary concern, we think, was the apparent need to think about all the cross-game interactions, like invaders worrying about asteroids. In practice, if we allow the class-specific interactions to default, that concern goes away, and any object from either game could be pretty quickly adapted to run in the other. There might be a need to coordinate between the teams if the shared class needed to interact with an object from its sibling game. That possibility is thought to be rare but it might happen. So far we have no real cases.

The teams agreed to pair on that issue if and when it actually comes up. They see no need to do work now, which would be speculative and quite possibly not needed at all.

Agreement

Our design here, like all designs, has some aspects that are not ideal. In our case, the need for class- vs class-specific interaction behavior sets up tensions in the code, centering on whether and how we use implementation inheritance.

We have decided that judicious use of implementation inheritance in the Flyer interfaces is allowed but not encouraged. Specifically, it is allowed to implement interact_with_newthing as pass in the interface rather than as @abstractmethod, though the latter is preferred.

We have further decided that implementing NewThing.interact_with as pass is too dangerous and that instead we should provide pass as the default implementation of interact_with_newthing. We plan to stop the old practice and to clean up existing cases over time.

This solution is not ideal: it doesn’t give us as much safety as it might. But in return for slightly reduced safety, we get an easier flow of work, with less impact of one object on another. It’s a very small shift in safety, since both teams were actually already walking on the grass. We’ve just blessed that behavior and decided that the pavement goes on the specific methods, not the general one.

Much of the impact of this decision is simply emotional. We used to be working around the system, violating what we took to be the rule: no implementation inheritance. Now, we do allow it, in these cases, following this pattern. The system is really no worse off: we were already breaking the rule. Now we have changed the rule, so we feel less tension, through knowing an approved way to do our work.

Is there some even better way? Perhaps. If we find it, we’ll try it. We may try the delegation idea, but that just trades one kind of inheritance for another. If we do try it, it’ll be more to learn how to do it, not with much expectation that it will actually be better. If we try it, we’ll find out. FAFO, in the best sense of the term.

Summary (TL/DR)

See you next time!