Python 76 - Avoid subclasses

While I am OK with the subclass design, I think we can do better. Also “Replace polymorphism with data.”

We currently have three kinds of explosion fragments, even with its own class: Fragment, which is just one line, VFragment(Fragment), which is two lines in a v-shape, and GFragment(Fragment), a little stick-man astronaut.

The Fragment class is the superclass for the other two, which override create_fragments. GFragment also overrides draw, so that it can draw the head of the astronaut, a circle. The Fragment draw just draws a collection of lines.

The good GeePaw Hill’s mother was, as I figure it, frightened by Implementation Inheritance while he was in the womb, so he has an absolute prohibition against it. In his reaction to it, I’ve seen things you people wouldn’t believe. When he’s done fighting Implementation Inheritance, it will be lost in time, like tears in the rain.

But he does have a point. Inheritance of behavior is a bit harder to understand, in that you generally need to look up and down the class hierarchy to sort it out. It’s probably easy to get it wrong. So, after his continual and dare I say quite incessant bickering about it, I have come to look for good ways to avoid the use of Implementation Inheritance.

Today we’ll reduce our three classes to one, and when we’re done, and if my guess is right, we’ll like it better. Let me share my plan with you. It goes roughly like this:

Tentative Plan

The “fragments” of each of our three types is a collection of pairs of points, representing one line of whatever is to be drawn. We’ll arrange to pass those collections in to Fragment when an instance is created, rather than build it on the fly.

We’ll provide factory creation methods for Fragment to create instances with one line, two for the v, or many for the stick man.

That will leave everything good except that no one will be drawing the astronaut’s head. VFragment and GFragment will be unused.

We’ll add a “command” to the Fragment’s fragments list, something like

["line", Vector2(...), Vector2(...)]

And then, finally, we’ll add a new command, “circle” or “head”.

After that, we should have the head back.

Idea

I’m glad we had this little chat. I think we can do all this without ever breaking the game, including the head, if we do the factory methods right. Let’s find out.

Plan Meets Code

class Fragment:
    def __init__(self, position, angle=None, speed_mul=None):
        angle = angle if angle is not None else random.randrange(360)
        self.position = position
        speed_mul = speed_mul if speed_mul is not None else random.uniform(0.25, 0.5)
        self.velocity = speed_mul*Vector2(u.FRAGMENT_SPEED, 0).rotate(angle)
        self.theta = random.randrange(0, 360)
        self.delta_theta = random.uniform(180, 360)*random.choice((1, -1))
        self.timer = Timer(u.FRAGMENT_LIFETIME, self.timeout)
        self.create_fragments()

Let’s add a fragments parameter to Fragment, and not call create_fragments if we get one:

class Fragment:
    def __init__(self, position, angle=None, speed_mul=None, fragments= None):
        angle = angle if angle is not None else random.randrange(360)
        self.position = position
        speed_mul = speed_mul if speed_mul is not None else random.uniform(0.25, 0.5)
        self.velocity = speed_mul*Vector2(u.FRAGMENT_SPEED, 0).rotate(angle)
        self.theta = random.randrange(0, 360)
        self.delta_theta = random.uniform(180, 360)*random.choice((1, -1))
        self.timer = Timer(u.FRAGMENT_LIFETIME, self.timeout)
        if fragments:
            self.fragments = fragments
        else:
            self.create_fragments()

Commit: provide for fragments parameter in Fragment init.

If our create_fragments methods returned the list, that would be better.

class Fragment:
    def __init__(self, position, angle=None, speed_mul=None, fragments= None):
        angle = angle if angle is not None else random.randrange(360)
        self.position = position
        speed_mul = speed_mul if speed_mul is not None else random.uniform(0.25, 0.5)
        self.velocity = speed_mul*Vector2(u.FRAGMENT_SPEED, 0).rotate(angle)
        self.theta = random.randrange(0, 360)
        self.delta_theta = random.uniform(180, 360)*random.choice((1, -1))
        self.timer = Timer(u.FRAGMENT_LIFETIME, self.timeout)
        if not fragments:
            self.fragments = self.create_fragments()

    def create_fragments(self):
        half_length = random.uniform(6, 10)
        begin = Vector2(-half_length, 0)
        end = Vector2(half_length, 0)
        return [[begin, end]]

class VFragment:
    def create_fragments(self):
        side_1 = [Vector2(-7, 5), Vector2(7, 0)]
        side_2 = [Vector2(7, 0), Vector2(-7, -5)]
        return [side_1, side_2]

class GFragment:
    def create_fragments(self):
        body_bottom = Vector2(0, 2)
        body = [Vector2(0, 16), body_bottom]
        left_leg = [Vector2(-5, -16), body_bottom]
        right_leg = [Vector2(5, -16), body_bottom]
        arm = [Vector2(-9, 10), Vector2(9, 10)]
        return [body, arm, left_leg, right_leg]

That’s good. Commit: create_fragments methods now return their list.

I think I’ll do the factory methods all at once. No, one at a time is better practice. We’ll wind up with factory methods simple_fragment, v_fragment, and astronaut_fragment. Let’s do simple:

    @classmethod
    def simple_fragment(cls):
        half_length = random.uniform(6, 10)
        begin = Vector2(-half_length, 0)
        end = Vector2(half_length, 0)
        return [[begin, end]]

    def __init__(self, position, angle=None, speed_mul=None, fragments= None):
        angle = angle if angle is not None else random.randrange(360)
        self.position = position
        speed_mul = speed_mul if speed_mul is not None else random.uniform(0.25, 0.5)
        self.velocity = speed_mul*Vector2(u.FRAGMENT_SPEED, 0).rotate(angle)
        self.theta = random.randrange(0, 360)
        self.delta_theta = random.uniform(180, 360)*random.choice((1, -1))
        self.timer = Timer(u.FRAGMENT_LIFETIME, self.timeout)
        if not fragments:
            self.fragments = self.create_fragments()

    def create_fragments(self):
        raise RuntimeError("should be unused")

I just moved the code from create_fragments up to simple_fragment. I have 8 tests breaking. I think they’re all the same, because I’m not yet using the factory method:

    def create_fragments(self):
>       raise RuntimeError("should be unused")
E       RuntimeError: should be unused

I hope I can do that. It didn’t occur to me to look at how I use these and now that I think about it, it’s going to be a bit tricky:

class ExplosionFleet:
    def explosion_at(self, position):
        fragment_classes = [VFragment, GFragment, Fragment, Fragment, Fragment, Fragment, Fragment]
        random.shuffle(fragment_classes)
        how_many = len(fragment_classes)
        for i in range(how_many):
            fragment_class = fragment_classes[i]
            base_direction = 360 * i / how_many
            self.make_fragment(fragment_class, position, base_direction)

    def make_fragment(self, fragment_class, position, base_direction):
        twiddle = random.randrange(-20, 20)
        fragment = fragment_class(position=position, angle=base_direction + twiddle)
        self.flyers.append(fragment)

Ah, right. Let’s cheat for now, by checking for the class in make_fragment. I think we’ll find that we can clean that up nicely once we’re ready.

    def make_fragment(self, fragment_class, position, base_direction):
        twiddle = random.randrange(-20, 20)
        if fragment_class == Fragment:
            fragment = Fragment.simple_fragment()
        else:
            fragment = fragment_class(position=position, angle=base_direction + twiddle)
        self.flyers.append(fragment)

Oh, I’ve done the class method wrong, it needs to return a Fragment. Silly me.

class Fragment:
    @classmethod
    def simple_fragment(cls, position, angle=None, speed_mul=None):
        half_length = random.uniform(6, 10)
        begin = Vector2(-half_length, 0)
        end = Vector2(half_length, 0)
        return cls(position, angle, speed_mul, [[begin, end]])

class ExplosionFeet:
    def make_fragment(self, fragment_class, position, base_direction):
        twiddle = random.randrange(-20, 20)
        if fragment_class == Fragment:
            fragment = Fragment.simple_fragment(position=position, angle=base_direction+twiddle)
        else:
            fragment = fragment_class(position=position, angle=base_direction + twiddle)
        self.flyers.append(fragment)

That has fixed 4 tests, but 4 are still borked. I’m tempted to revert but let’s see what’s wrong first. The tests are all like this:

    def test_frag_timeout(self):
        frag = Fragment(position = u.CENTER)
        fleets = Fleets()
        frags = fleets.explosions
        frags.append(frag)
        frags.tick(0.1, fleets)
        assert frags
        frags.tick(u.FRAGMENT_LIFETIME, fleets)
        assert not frags

I think that if I were to remove my raise and return an empty list, these would all work. They’re checking other aspects of Fragment.

class Fragment:
    def create_fragments(self):
        return []

Green. Check game. Arrgh.

  File "/Users/ron/PycharmProjects/firstGo/fragment.py", line 35, in draw
    self.draw_lines(screen, self.position, self.theta, self.fragments)
                                                       ^^^^^^^^^^^^^^
AttributeError: 'Fragment' object has no attribute 'fragments'

I’m going to figure out what this is but i know I should just revert and do again.

I think I know. It’s the truthiness again.

    def create_fragments(self):
        print("in create fragments")
        return [[Vector2(-5, 0), Vector2(5, 0)]]

That does not fix it. How am I getting in here without a fragments member?

Oh. I don’t save it if I receive it as parameter. I swear I typed in the else below. I must have then thought it was unneeded.

    def __init__(self, position, angle=None, speed_mul=None, fragments=None):
        angle = angle if angle is not None else random.randrange(360)
        self.position = position
        speed_mul = speed_mul if speed_mul is not None else random.uniform(0.25, 0.5)
        self.velocity = speed_mul*Vector2(u.FRAGMENT_SPEED, 0).rotate(angle)
        self.theta = random.randrange(0, 360)
        self.delta_theta = random.uniform(180, 360)*random.choice((1, -1))
        self.timer = Timer(u.FRAGMENT_LIFETIME, self.timeout)
        if not fragments:
            self.fragments = self.create_fragments()
        else:
            self.fragments = fragments

We’re good now. Commit: Fragment instances created with class method.

I really should fix those tests so that I can remove my create methods as I no longer need them. I just add a fake fragments parameter in the tests:

    def test_frag(self):
        frag = Fragment(position = u.CENTER, fragments=["ignored"])
        assert frag

I do that in all the places. Green. Commit: Tests now provide fake fragments collections.

Reflection

OK, that got a little ragged, but the real issue was just that I missed out the else on the fragment assignment. The tests that needed fixing were just how things go, sometimes when we change how things are used we have to fix up some tests.

Now let’s do the V fragment. We’ll add another class method.

    @classmethod
    def v_fragment(cls, position, angle=None, speed_mul=None):
        side_1 = [Vector2(-7, 5), Vector2(7, 0)]
        side_2 = [Vector2(7, 0), Vector2(-7, -5)]
        return cls(position, angle, speed_mul, [side_1, side_2])

Some more tests fail and I need to change the creation:

class ExplosionFleet:
    def make_fragment(self, fragment_class, position, base_direction):
        twiddle = random.randrange(-20, 20)
        if fragment_class == Fragment:
            fragment = Fragment.simple_fragment(position=position, angle=base_direction+twiddle)
        elif fragment_class == VFragment:
            fragment = Fragment.v_fragment(position=position, angle=base_direction+twiddle)
        else:
            fragment = fragment_class(position=position, angle=base_direction + twiddle)
        self.flyers.append(fragment)

One test still fails. Ah, interesting. It’s the one that tries to show that we get five regular Fragments and one each V and G. I think that test is doomed:

    def test_explosion_fleet(self):
        fleet = ExplosionFleet()
        explosion = fleet.flyers
        fleet.explosion_at(u.CENTER)
        assert len(explosion) == 7
        frags = [fragment for fragment in explosion if type(fragment) == Fragment]
        assert len(frags) == 5
        frags = [fragment for fragment in explosion if type(fragment) == VFragment]
        assert len(frags) == 1
        frags = [fragment for fragment in explosion if type(fragment) == GFragment]
        assert len(frags) == 1

By the time we’re done, we won’t have those classes to check. I think we have little choice but to remove this test. We’ll see if it makes me nervous.

We’re green and good. Commit: V-shaped Fragments now drawn by Fragment.

We can’t get rid of the VFragment class yet, because of our test on class in the ExplosionFleet. We’ll sort that as a separate step.

Now let’s do the astronaut. My plan is to cheat a bit:

class Fragment:
    @classmethod
    def astronaut_fragment(cls, position, angle=None, speed_mul=None):
        body_bottom = Vector2(0, 2)
        body = [Vector2(0, 16), body_bottom]
        left_leg = [Vector2(-5, -16), body_bottom]
        right_leg = [Vector2(5, -16), body_bottom]
        arm = [Vector2(-9, 10), Vector2(9, 10)]
        return GFragment(position, angle, speed_mul, [body, arm, left_leg, right_leg])

class ExplosionFleet:
    def make_fragment(self, fragment_class, position, base_direction):
        twiddle = random.randrange(-20, 20)
        if fragment_class == Fragment:
            fragment = Fragment.simple_fragment(position=position, angle=base_direction+twiddle)
        elif fragment_class == VFragment:
            fragment = Fragment.v_fragment(position=position, angle=base_direction+twiddle)
        else:
            fragment = Fragment.astronaut_fragment(position=position, angle=base_direction + twiddle)
        self.flyers.append(fragment)

With this in place, we’re still creating the GFragment instance, but by calling the correct class method on fragment. We’re green and good. Commit: Astronaut fragment created by Fragment.astronaut_fragment, still using GFragment class until head is handled.

Now what?

I think we can remove a lot of code from the classes, but we can’t remove everything. Let’s change the code in ExplosionFleet.

    def explosion_at(self, position):
        fragment_classes = [VFragment, GFragment, Fragment, Fragment, Fragment, Fragment, Fragment]
        random.shuffle(fragment_classes)
        how_many = len(fragment_classes)
        for i in range(how_many):
            fragment_class = fragment_classes[i]
            base_direction = 360 * i / how_many
            self.make_fragment(fragment_class, position, base_direction)

    def make_fragment(self, fragment_class, position, base_direction):
        twiddle = random.randrange(-20, 20)
        if fragment_class == Fragment:
            fragment = Fragment.simple_fragment(position=position, angle=base_direction+twiddle)
        elif fragment_class == VFragment:
            fragment = Fragment.v_fragment(position=position, angle=base_direction+twiddle)
        else:
            fragment = Fragment.astronaut_fragment(position=position, angle=base_direction + twiddle)
        self.flyers.append(fragment)

I think I want to change the fragment_classes list to be fragment_factory_methods and use that, passing the method down to make_fragment.

    def explosion_at(self, position):
        simple = Fragment.simple_fragment
        vee = Fragment.v_fragment
        guy = Fragment.astronaut_fragment
        fragment_factory_methods = [vee, guy, simple, simple, simple, simple, simple]
        random.shuffle(fragment_factory_methods)
        how_many = len(fragment_factory_methods)
        for i in range(how_many):
            factory_method = fragment_factory_methods[i]
            base_direction = 360 * i / how_many
            self.make_fragment(factory_method, position, base_direction)

And then:

    def make_fragment(self, factory_method, position, base_direction):
        twiddle = random.randrange(-20, 20)
        fragment = factory_method(position=position, angle=base_direction+twiddle)
        self.flyers.append(fragment)

Green and good. Commit: ExplosionFleet converted to use Fragment factory methods.

I thought we might want to inline that last bit, but for now, we’ll let it be.

I think we can now remove the VFragment class entirely and most of GFragment. Right. VFragment is removed and this is GFragment:

class GFragment(Fragment):
    def __init__(self, position, angle=None, speed_mul=None, fragments=None):
        super().__init__(position, angle, speed_mul, fragments)

    def draw(self, screen):
        super().draw(screen)
        self.draw_head(screen)

    def draw_head(self, screen):
        head_off = Vector2(0, 16 + 8).rotate(self.theta)
        pygame.draw.circle(screen, "white", self.position + head_off, 8, 2)

Let’s take a breath here. In fact, we’ll sum up here and then do the next steps in a new article.

Summary

In seven commits over a bit less than two hours, we’re well on the way to removing the nasty subclasseses we hates them supporting different kinds of fragments. The next steps will be to turn the pairs of points into some kind of little command language. Whether we’ll build a class for that, I’m not sure. I’m sure we won’t start that way, unless I change my mind before the next article.

It went well, with momentary confusion because I flat forgot to save the fragments when they were provided as a parameter. Once I tipped to that, the code was obviously right and we went back into our change-commit cycle.

I was asking myself,

Self, should we have just done this to begin with?

And I answered,

Well, self, ‘Nemo dat quod non habet’, so I figure we moved toward something good, got to a good enough place, and from there we could see further. We don’t always go straight to perfect. In fact I don’t think we ever have.”

We try always to move from good to better, over and over. It’s not a straight path, as Hill teaches us. We wander a bit and that’s the nature of creative work.

Our VFragment class is completely removed. GFragment will be removed soon. We are replacing each class with a single factory method. Rather nice, actually.

See you next time! I’m going to press right on to number 77. I’m on a roll.