When is it appropriate to introduce a new layer of abstraction into a class hierarchy?

Question

Suppose I'm creating a game played on a 2D coordinate grid. The game has 3 types of enemies which all move in different ways:

• Drunkard: moves using type 1 movement.
• Mummy: moves using type 1 movement, except when it's near the main character, in which case it will use type 2 movement.
• Ninja: moves using type 3 movement.

Here are the ideas I've come up with in organizing the class hierarchy:

Proposal 1

A single base class where each enemy is derived from:

abstract class Enemy:
    show()   // Called each game tick
    update() // Called each game tick
    abstract move() // Called in update

class Drunkard extends Enemy:
    move() // Type 1 movement

class Mummy extends Enemy:
    move() // Type 1 + type 2 movement

class Ninja extends Enemy:
    move() // Type 3 movement

Problems:

• Violates DRY since code isn't shared between Drunkard and Mummy.

Proposal 2

Same as proposal 1 but Enemy does more:

abstract class Enemy:
    show()            // Called each game tick
    update()          // Called each game tick
    move()           // Tries alternateMove, if unsuccessful, perform type 1 movement
    abstract alternateMove() // Returns a boolean

class Drunkard extends Enemy:
    alternateMove(): return False

class Mummy extends Enemy:
    alternateMove() // Type 2 movement if in range, otherwise return false

class Ninja extends Enemy:
    alternateMove() // Type 3 movement and return true

Problems:

• Ninja really only has one move, so it doesn't really have an "alternate move." Thus, Enemy is a subpar representation of all enemies.

Proposal 3

Extending proposal 2 with a MovementPlanEnemy.

abstract class Enemy:
    show()   // Called each game tick
    update() // Called each game tick
    abstract move() // Called in update

class MovementPlanEnemy:
    move() // Type 1 movement
    abstract alternateMove()

class Drunkard extends MovementPlanEnemy:
    alternateMove() // Return false

class Mummy extends MovementPlanEnemy:
    alternateMove() // Tries type 2 movement

class Ninja extends Enemy:
    move() // Type 3 movement

Problems:

• Ugly and possibly over-engineered.

Question

Proposal 1 is simple but has a lower level of abstraction. Proposal 3 is complex but has a higher level of abstraction.

I understand the whole thing about "composition over inheritance" and how it can solve this whole mess. However, I have to implement this for a school project which requires us to use inheritance. So given this restriction, what would be the best way to organize this class hierarchy? Is this just an example of why inheritance is inherently bad?

I guess since my restriction is that I have to use inheritance, I'm really asking the broader question: in general, when is it appropriate to introduce a new layer of abstraction at the cost of complicating the program architecture?

Answer 1

I've built a 2D roguelike from pretty much scratch, and after lots of experimentation, I used an entirely different approach. Essentially an entity component architecture.

Each game object is an Entity, and an Entity has many attributes which control how it responds to stimuli from the player and the environment. One of these components in my game is a Movable component (other examples are Burnable, Harmable, etc, my GitHub has the full list):

class Entity
    movable
    harmable
    burnable
    freezable
    ...

Different types of enemies are distinguised by injecting different basic components at object creation time. So something like:

drunkard = Entity(
    movable=SometimesRandomMovable(),
    harmable=BasicHarmable(),
    burnable=MonsterBurnable(),
    freezable=LoseATurnFreezable()
    ...
)

and

ninja = Entity(
    movable=QuickMovable(),
    harmable=WeakHarmable(),
    burnable=MonsterBurnable(),
    freezable=NotFreezable()
    ...
)

Each component stores a reference to its owner Entity for information like position.

The components know how to receive messages from the game world, process them, then generate more messages for the results. These messages land in a global queue, and there is a main loop each turn which pops messages off the queue, process them, then pushes and resulting messages back on the queue. So, for example, a Movable component does not actually edit the position attributes of the owning entity, it generates a message to the game engine that they should be changed, along with the position in which the owner should be moved to.

There's essentially no class hierarchy for the basic game entities, and I did not find myself missing it. Behavior is distinguished entirely by what components a entity has. This works for every entity in the game world, player, enemy, or object.

Answer 2

This is why we often like interfaces over inheritance: Many real-world problems cannot be modeled in an object hierarchy.

interface IMove
{
    // returns an intermediate location chosen with 
    // the intention to move toward destination
    Point Move(Point currentLocation, Point destination)
}

Now we can inject an IMove, or we can write a "move this object using strategy ninja" type of function.

We can also test the movement strategies separately

Answer 3

I would follow your first option, but then use the strategy pattern for the different move styles. This will allow you to swap move styles and alter moves styles easier moving forward.

So you’d have an interface called MoveStyle and then several classes implementing it for each kind of movement.

Answer 4

The answer to your main question:

in general, when is it appropriate to introduce a new layer of abstraction at the cost of complicating the program architecture?

is relatively simple and straightforward:

When the benefit outweighs the cost

You need to be certain about the benefits and costs involved. An additional abstraction layer will not complicate a project with 3 classes too much, but it might be a deal-breaker in a project where dozens of classes would be affected. Refactoring logical abstractions into a model is very much nontrivial and needs to be carefully weighed against the benefits.

As far as the benefits are concerned, the two broad aspects I would be looking after are high expressive power, as well as high adaptability. In short, if it makes your code more expressive, that's a plus. If it makes it flexible to easily adapt to forthcoming unforeseen requirements, that's an even bigger plus!

Do not underestimate the expressive power, because that's where adaptability also hides. The better you understand and "mimic" your modeled domain, the better you can "foresee" potential future requirements.

Also

Depending on your perspective, you might be confusing the true meaning of abstraction. Abstractions are powerful, because they hide details. You state:

Proposal 1 is simple but has a lower level of abstraction. Proposal 3 is complex but has a higher level of abstraction

No, it's the other way around. Your "Proposal 1" has a higher level of abstraction compared to "Proposal 3". Take a step outside of where you stand. You know nothing about your design, and you present it to others.

From your "Proposal 1", I immediately know you have some kind of entity termed Enemy which you can show, update and move. Plus you have some specific types of Enemy.

From your "Proposal 3", I know, additionally, that there is a special type of Enemy, called however you like, e.g. MovementPlanEnemy. Some of your types implement this instead of the basic Enemy type, so now, I know more about your types. You are becoming more specific, you offer two types of movement, plain and alternative.

Think about where you are going to use these types. Within your game, you will, eventually, have to declare the general base types, so that you can compose your logic. Wherever you declare your types as MovementPlanEnemy, you are "leaking" more details than if you would declare them as Enemy. When I know more about your design, you go down the ladder of abstraction, towards "concretion". When you do that, you usually lose expressive power because you have less flexibility. You now offer more information and this complicates things more as you will have to support these specific provisions of yours in future versions of your code (or force everyone using them, including you, to change their code to adapt to your new decisions).

Also (part II)

For your "Proposal 1", you state:

Violates DRY since code isn't shared between Drunkard and Mummy.

You might be being somewhat too strict with your assessment there. Of course code will not be shared between Drunkard and Mummy. DRY (Do not Repeat Yourself) does not mean do not write identical lines of code. It means, strive to reuse your concepts. Type 1 Movement is a concept and you can avoid repeating yourself regarding that quite easily by, for example, using static helper classes, or, as others have stated, through composition and interfaces (strategy pattern and whatnot). But tomorrow, you may be required to add another character that moves like two already existing characters. Would you change the abstraction again, in order to separate movement types into more groups?

In short, you don't need to change your entire model just to accommodate the fact that some of your classes share part of their behavior. The reason for this is that, in your case, the common code is most likely a coincidence and not an actual design detail decision. You need characters that move on a 2D-grid, each in its own way, based on the circumstances. Your other proposals, implying characters that can move in two different ways (at most), will also necessitate the actual support of two ways at most and, suddenly, you will find yourself making additional checks throughout the main game logic, to find out how your characters should move (i.e. which method to call, move() or alternativeMove()). The decision about how characters should move is best contained (encapsulated) within the character classes, potentially aided by supplying additional details (preferably through the constructor of each character). The only model that supports this design choice is your "Proposal 1" and you should keep this and try to solve the rest of your problems without sacrificing its high abstraction level.

Answer 5

If you make Mummy extend Drunkard (you could argue it's a slightly smarter drunkard) instead of Enemy, your conditional can either call the base (i.e. Drunkard) move() or use type 2 movement. You can see this as a variant on Proposal 3, in which there's no alternateMove(), and Drunkard serves the role you gave to the MovementPlanEnemyclass. That class's name, incidentally, is more suggestive of the strategy-class approach in @AdamB's answer.

Another way to prevent DRY violation is to make a type 1 movement method live outside all enemies' classes, and have Drunkard and Mummy call it as needed. Depending on how you implement this approach, it can reduce to @AdamB's idea to also have a MoveStyle class. You don't have to create yet another class, but the type-1 movement method has to live somewhere.

As @Gqqnbig noted, the first of these suggestions has one big downside: maintenance. It only "works" in the YAGNI sense that we don't currently need Mummy to differ much from Drunkard, so such an inheritance respects the spirit of our current needs. Mummy inheriting from Drunkard may lead to many more overrides being needed in the long term. Since this is for a game, the most obvious issue is we'll eventually add audiovisual indicators of the enemy type. That's probably why they call it YAGNI rather than YDNIRN (You Don't Need It Right Now).

Answer 6

You're overusing object hierarchies.

(For a bit of background reading: An execution in the kingdom of the nouns, by Steve Yegge.)

You've created an object hierarchy of Enemy's. That's a perfectly fine thing to do in itself, when there's enough shared data and functionality among all Enemies. However, into this hierarchy you have shoved functionality which doesn't reasonably belong in it: show(), update(), move().

These methods:

• Are mostly not from the game's conceptual space (Mummies don't update themselves, and drunkards don't switch from being not-shown to shown).
• Require information, and affect state, which is not part of the Enemy object.
• Do not specialize in accordance with the class hierarchy - which is what you noticed: A Mummy and a Drunkard partially share a kind of movement.
• Likely don't need access to the Enemy objects' private members (/protected members), and can make do with the pubic fields and methods.

Many answers here suggest you use even more objects, more nouns, complex combinations of interfaces or aspects or abstract base classes which can capture the commonality of Mummy and Drunkard movement. But - I'd say that this kind of shoe-horning is a bad idea.

IMHO, you need to let go of the assumption that game logic belongs in class definitions, and you have to force it in there if it doesn't fit.

Focusing on movement - you should seriously consider:

• Making move() a freestanding function, or a method of your Board class etc., which takes an Enemy as a parameter.
• Collecting most of your "move logic" into that function.
• If you really need a lot of flexibility, having a helper function, say, resolve_movement_type(), which takes an Enemy and some state information as parameters (e.g. distance from player, or time-of-day and so on) etc. Then you can call a more specialized movement routine - type 1, 2 or 3.

and perhaps something similar in spirit for show() and update().

Answer 7

Note: You forgot to mention preferred P.L.

Conceptually speaking, each Enemy has a single movement, either simple or complex so, lets start with :

abstract class Enemy:
  show( )
  update( )
  move ( )

Let's add the subclasses, for the moment, just think, every class move is just different.

class Drunkard: extends Enemy
  /* override */ move ( )

class Mummy: extends Enemy
  /* override */ move ( )

class Ninja: extends Enemy
  /* override */ move ( )

Ok, we already know move can be mixed or simple, and simple may not be used for all subclasses.

There are two ways to deal with it. One is to add the simple methods as protected methods like:

abstract class Enemy:
  show( )
  update( )
  move ( )
  /* protected */ simplemove1 ( )
  /* protected */ simplemove2 ( )
  /* protected */ simplemove3 ( )

And, each subclass move method calls the required simple methods. But, this will not help, if we want to add more enemies, and movements.

Another way is to use "traits", similar to "interfaces", but its not implemented by several P L.

The third option is, as you already know, to delegate the move operation to another class.

abstract class MoveOperation:
  move ( )

abstract class Enemy:
  show( )
  update( )
  move ( )

And, it does add an extra, but still valid, layer.

For a moment, lets suppouse movements are simple.

abstract class MoveOperation:
  abstract move ( )

class MoveOperation1: extends MoveOperation
  abstract move ( )

class MoveOperation2: extends MoveOperation
  abstract move ( )

class MoveOperation3: extends MoveOperation
  abstract move ( )

Then each overriden move( ) operation will create and call the required method.

Ninja.move ( ):
    MoveOperation M = new MoveOperation1( )
    M.move( )

But, since you can combine those, then lets create a simple class and a composed class:

abstract class SimpleMove:
  abstract move ( )

class MoveOperation1: extends SimpleMove
  move ( )

class MoveOperation2: extends SimpleMove
  move ( )

class MoveOperation3: extends SimpleMove
  move ( )

And then, the composed operation:

abstract class ComposedMove:
  abstract move ( )

class DrunkardMove: extends ComposedMove
  move ( )

class MommyMove: extends ComposedMove
  move ( )

class NinjaMove: extends ComposedMove
  move ( )

NinjaMove.move:
  SimpleMove1 M1 = new SimpleMove1( )
  SimpleMove2 M2 = new SimpleMove2( )
  M1()
  M2()

And the Enemy class provides:

abstract class Enemy:
  /* protected */ ComposedMove Action

  show( )
  update( )
  abstract move ( )

class Drunkard: extends Enemy
  /* override */ move ( )

class Mummy: extends Enemy
  /* override */ move ( )

class Ninja: extends Enemy
  /* override */ move ( )

Ninja.move ( ):
    this.Action = new NinjaMove( )
    this.Action.move( )

And the "Beauty" of this, is that you can add later, other "Enemies":

class EvilKittyMove: extends ComposedMove
  move ( )

class EvilKitty: extends Enemy
  /* override */ move ( )

Cheers.

Answer 8

From where mainCharacter comes from?

From your description, move() sometimes depend on data from main character, sometimes not.

In this situation, mainCharacter must be a parameter of move() in the interface. It's a implementation detail in derived class if it is used or not.

If mainCharacter is a global or contextual data in your game engine, so any code can reach, then it's a implementation detail of Mummy's move() to:

class Mummy extends Enemy:
    move()
        if ( self.IsNear( context.MainCharacter ) )
            moveType2()
        else
            moveType1()

In a game engine, you probably return a composite object containing:

class CalculatedMove
    var Sprint
    var Location

class Mummy extends Enemy:
    move()
        return new CalculatedMove( self , calculateNextPositoin( context.MainCharacter ) )

Answer 9

The user @Vector makes an important point which is obscured by his lenghty answer: DRY does not mean do not write identical lines of code.

With that said, Proposal 1 is clearly preferred, but it's potentially incomplete.

Your concern over duplicating the code for moves using type 1 movement can be addressed in at least a couple of different ways:

Option 1. Don't worry about it, go ahead and duplicate it. As your code evolves to accommodate future requirements, what are the chances that the movement for Drunkard and Mummy will remain exactly the same forever? In this case, having duplicate lines of code may prevent the introduction of future bugs where a change to one breaks the other.

Option 2. More appropriate if the movement related code is non-trivial: create a separate class hierarchy for Movement and make it an attribute of Enemy. Now you can re-use code within the Movement class:

TL;DR

abstract class Movement:
    abstract move() // Called by Enemy code to move itself

abstract class Enemy:
    show()   // Called each game tick
    update() // Called each game tick
    abstract movement() // Called in update

class Drunkard extends Enemy:
    movement(): return new Type1Movement

class Mummy extends Enemy:
    movement(): if isMainCharacter return new Type1Movement else return new Type2Movement

class Ninja extends Enemy:
    movement(): return new Type3Movement

class Type1Movement extends Movement:
    move(): ... your code here ...

class Type2Movement extends Movement:
    move(): ... your code here ...

class Type3Movement extends Movement:
    move(): ... your code here ...

Answer 10

It's nice when the code resembles the spec.

Here's the spec again:

Drunkard: moves using type 1 movement.

Mummy: moves using type 1 movement, except when it's near the main character, in which case it will use type 2 movement.

Ninja: moves using type 3 movement.

And here's a way to write the code so it resembles it:

enum EnemyType {
    Drunkard,
    Mummy,
    Ninja,
}

class Enemy
    EnemyType enemyType
    move() {
        switch (enemyType) {
            case Drunkard:
                movetype1()
                break
            case Mummy:
                if closeToPlayer()
                    movetype2()
                else
                    movetype1()
                break
            case Ninja:
                movetype3()
                break
        }
    }