Python Asteroids on GitHub

At least I’m trying …

On a private channel, Hill and I had this exchange:

Ron
So instead of subscribe if you wanna, which is hard to get right when adding a thing, what if in the thing code we could specify the classes that we think should interact with it. Naturally we could always think about that. But if you had to declare it … hmm.
GeePaw Hill
Worth a shot. My rule: “It’s hardly gonna be any worse than what I tried yesterday, and at least I’m trying, which is a merit all its own.”

So, that’s my idea. What if a requirement / common practice with these Flyers is that when we write one, we ask it or require it to implement a special class method naming the classes that it thinks need to interact with it. And then, in a test, we iterate the Flyer subclasses, fetching that list, and check each class that it mentions to ensure that it implements interact_with_....

That way, we’d perhaps be able to continue to use our default pass methods but be more sure that we have implemented the key interactions.

Now, I still believe, newly based on sad experience, that simply making all the interact_with... methods abstract, so that everyone is required to implement them, is probably the safest thing to do. But it is a pain, in that every time we add a Flyer subclass, we have to edit every other Flyer subclass. That’s quite irritating, as there are something like 14 of them.

So, as Hill says, at least I’m trying.

Let’s write the test.

    def test_should_implement(self):
        subclasses = get_subclasses(Flyer)
        for klass in subclasses:
            required_method = "interact_with_" + klass.__name__.lower()
            try:
                should_interact = klass.should_interact_with()
            except AttributeError:
                should_interact = []
            for interactor in should_interact:
                try:
                    method = interactor.__dict__[required_method]
                except AttributeError:
                    assert False, interactor.__name__ + " does not implement " + required_method

A bit odd, but enough to try it. The test only checks classes that actually implement the new should_interact_with method.

Let’s try it somewhere. I’ll test it in GameOver which actually has no interactions. It would be legitimate for it not even to implement interact_with, but them’s the rules.

class GameOver(Flyer):      
    @classmethod
    def should_interact_with(self):
        return [Ship]

One test fails, as intended, saying:

>                   method = interactor.__dict__[required_method]
E                   KeyError: 'interact_with_gameover'

My bad, I checked for the wrong exception. I also find that pytest intercepts all exceptions, which results in a confusing message, so I recast the test and rename it:

    def test_should_interact_with(self):
        # a subclass xyz of Flyer can implement
        # should_interact_with to return a list of classes
        # each of which will be checked for implementing
        # interact_with_xyz
        subclasses = get_subclasses(Flyer)
        for klass in subclasses:
            required_method = "interact_with_" + klass.__name__.lower()
            if "should_interact_with" in klass.__dict__:
                should_interact = klass.should_interact_with()
                for interactor in should_interact:
                    if required_method not in interactor.__dict__:
                        assert False, interactor.__name__ + " does not implement " + required_method

Very nested but maybe we’ll refactor it later. The test now fails as intended:

AssertionError: Ship does not implement interact_with_gameover

So, that works nicely. I’ll roll back GameOver to remove the code I used to test the test, and then commit: test_should_interact_with allows Flyer subclasses to specify what other classes should interact with them.

Note that this “feature” is in addition to the practice of making interact_with_xyz methods @abstract, which is better, in that the compiler requires the method to exist in all FLyer subclasses, and rather a blunt tool, in that it requires the method to exist even in subclasses that legitimately have no interest in instances of xyz.

I see no ideal balance between reliability, which will result only from implementing all the necessary interact_with_xyz methods correctly, and clarity, which is damaged by having all the redundant pass methods in classes that do not need the method, and convenience, which is aided by allowing the Flyer superclass to default all such methods to pass, allowing us to implement real code only where it is needed.

But I think we have the problem mostly surrounded now, and this scheme is at least a reasonable thing to try.

Let’s put the method in place for a class or two. How about ScoreKeeper?

Oh. This is interesting. ScoreKeeper wants to interact with Score, and has that method:

class ScoreKeeper(Flyer):

    def interact_with_shipmaker(self, shipmaker, fleets):
        self._ship_maker = shipmaker

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

Now here we are, thinking about what ScoreKeeper should say in its should method.

Should it ask that ShipMaker interact with it? No, it’s just grabbing the maker to get some info from it. But how about Score? As the designers, we know that once ScoreKeeper has tallied the score from a Score, we want that Score to destroy itself. So we should implement this:

class ScoreKeeper(Flyer):
    @classmethod
    def should_interact_with(self):
        return [Score]

You know what would be nice? It would be nice if the information here could say something like this, that could come out in the pytest error message:

class ScoreKeeper(Flyer):
    @classmethod
    def should_interact_with(self):
        return [Score: "should delete itself"]

We’ll consider that later, but it would provide better documentation of why we insist that the other class implement the method.

Now, I just asked myself why, given that I was about to type in the should_interact_with, I didn’t just go over to Score and do the thing right then. And I probably should. But the point of this declaration is to force me to think. I believe that what I should do, is to require every Flyer to implement should_interact_with.

It’s early days for this idea and it’s about 5 AM, so I am not willing to make that commitment, not least because it will again force me to implement 14 methods in 14 classes.

We’ll let it ride for now, as an interesting idea that just might work.

At least I’m trying!

Added in Post
Won’t work quite as I intended. If ScoreKeeper and Score are supposed to interact, my scheme would give ScoreKeeper a reference to Score and Score a reference to ScoreKeeper and pytest or Python won’t let a recursive import go through. I shall have to look up how one deals with that.

Ah. Can do this:

    @classmethod
    def should_interact_with(self):
        from scorekeeper import ScoreKeeper
        return [ScoreKeeper]

Somewhat of a hack but it works. We’ll make that the convention and try it out for a while. Might be a good idea, might not. We’ll see.

See you next time!