1

I built a pygame game a few years back. It worked, but wasn't the best coding style and had a lot of classic code smells. I've recently picked it back up and am trying to refactor it with more discipline this time.

One big code smell was that I had a huge class, GameObject, that inherited from pygame.sprite.DirtySprite which had a lot of code related to:

  • various ways of moving a sprite
  • various ways of animating a sprite
  • various ways of exploding a sprite
  • etc.

The crazier I though of ways for sprites to behave, the code duplication was adding up and changes were getting more difficult. So, I started breaking out functionality into lots of smaller classes and then passing them in at object creation:

class GameObject(DirtySprite):
    def __init__(initial_position, mover_cls, imager_cls, exploder_cls):
        self.mover = mover(self, initial_position)
        self.imager = imager(self)
        self.exploder = exploder(self)

...
spaceship = GameObject(pos, CrazyMover, SpaceshipImager, BasicExploder)

As I factored out more and more code into these helper classes, the code was definitely better, more flexible and had less duplication. However, for each type of helper classes, the number of parameters got longer and longer. Creating sprites became a chore and the code was ugly. So, during another refactor I created a bunch of really small classes to do the composition:

class GameObjectAbstract(MoverAbstract, ImagerAbstract, \
                         ExploderAbstract, DirtySprite):
    def __init__(self, initial_position):
        ...
...
class CrazySpaceship(CrazyMover, SpaceshipImager, BasicExploder, GameObjectAbstract):
    pass  # Many times, all the behavior comes from super classes

...
spaceship = CrazySpaceship(pos)

I like this approach better. Is this a common approach? It seems to have the same benefits of having all the logic broken out in small classes, but creating the objects is much cleaner.

However, this approach isn't as dynamic. I cannot, for example, decide on a new mashup at run-time. However, this wasn't something I was really doing. While I do a lot of mashups, it seems OK that they are statically defined using class statements.

Am I missing anything when it comes to future maintainability and reuse? I hear that composition is better than inheritance, but this feels like I'm using inheritance to do composition - so I feel like this is OK.

Is there a different pattern that I should be using?

RobertB
  • 1,879
  • 10
  • 17

1 Answers1

2

That is ok, if you can separate the behaviors well enough -

Just that it is not "composition" at all - it is multiple inheritance, using what we call "mixin classes": a mixin class is roughly a class that provides an specific behavior that can be combined with other classes.

If you are using Python's super correctly, thatcouldbe the best approach. (If you are managing to create your game objects basically just defining the class name and the mixin classes it uses, that is actually a very good approach)

By the way, if you ever want to create new classes at runtime with this method, it is also possible - just use a call to type to create a new class, instead of a class statement: 

class CrazySpaceship(CrazyMover, SpaceshipImager, BasicExploder, GameObjectAbstract):
    pass  # Many times, all the behavior comes from super classes

Is just equivalent in Python to:

CrazySpaceShip = type('CrazySpaceShip', (CrazyMover, SpaceshipImager, BasicExploder, GameObjectAbstract), {})

And the tuple you used as second parameter can be any sequence built at runtime.

jsbueno
  • 99,910
  • 10
  • 151
  • 209
  • Cool tip. I was thinking that it was probably possible if I needed to, I'll put that in my toolbox just in case. Thanks also for the validation of the approach. I guessed I was struggling with the terminology somewhat. – RobertB Apr 30 '17 at 05:55
  • And as for using `super()` correctly. I decided upon two patterns. (1) E.g. the `Mover.move()` method must be implemented by a concrete class. If you called `spaceship.move()` only one Mover should handle it and if no Mover's handle it you get a "NotImplementedError" exception. (2) All mixins may implement a `process()` method that is expected to call `super().process()`. So calling `spaceship.process()` will mean that **all** the mixin classes get called and there is no "NotImplementedError" if you have none. Every time through the game loop, I call both. So... am I doing that right? – RobertB Apr 30 '17 at 17:37