When to Subclass instead of differentiating the behaviour

Question

I'm having difficulties deciding when I should be subclassing instead of just adding an instance variable that represents different modes of the class and then let the methods of the class act according to the selected mode.

For example, say I've a base car class. In my program I'll deal with three different types of cars. Race cars, busses and family models. Each will have their own implementation of gears, how they turn and seat setup. Should I subclass my car into the three different models or should I create a type variable and make the gears, turning and seating generic so they would act different depending on which car type was selected?

In my current situation I'm working on a game, and I've come to realise that it's starting to get a bit messy, so I ask advice on possibly refactoring of my current code. Basically there are different maps, and each map can be one of three modes. Depending on which mode the map is defined as there will be different behaviour and the map will be built in a different way. In one mode I might have to give out rentals to players and spawn creatures on a timeout basis, wherein another the player is responsable for spawning the creatures and yet in another there might be some automated spawned creatures alongside with player spawned ones and players constructing buildings. So I'm wondering whether it would be best to have a base map class, and then subclass it into each of the different modes, or whether to continue down my current path of adding differentiated behaviour depending on what the map type variable is set to.

Answer 1

All credits to AtmaWeapon of http://www.xtremevbtalk.com answering in this thread (Archive Link)

Core to both situations is what I feel is the fundamental rule of object-oriented design: the Single Responsibility Principle. Two ways to express it are:

"A class should have one, and only one, reason to change."
"A class should have one, and only one, responsibility."

SRP is an ideal that can't always be met, and following this principle is hard. I tend to shoot for "A class should have as few responsibilities as possible." Our brains are very good at convincing us that a very complicated single class is less complicated than several very simple classes. I have started doing my best to write smaller classes lately, and I've experienced a significant decrease in the number of errors in my code. Give it a shot for a few projects before dismissing it.

I first propose that instead of starting the design by creating a map base class and three child classes, start with a design that separates the unique behaviors of each map into a secondary class that represents generic "map behavior". This post is concerned with proving this approach is superior. It is hard for me to be specific without a fairly intimate knowledge of your code, but I'll use a very simple notion of a map:

Public Class Map
    Public ReadOnly Property MapType As MapType
    
    Public Sub Load(mapType)
    Public Sub Start()
End Class

MapType indicates which of the three map types the map represents. When you want to change the map type, you call Load() with the map type you want to use; this does whatever it needs to do to clear the current map state, reset the background, etc. After a map is loaded, Start() is called. If the map has any behaviors like "spawn monster x every y seconds", Start() is responsible for configuring those behaviors.

This is what you have now, and you are wise to think it's a bad idea. Since I mentioned SRP, let's count the responsibilities of Map.

• It has to manage state information for all three map types. (3+ responsibilities*)
• Load() has to understand how to clear the state for all three map types and how to set up the initial state for all three map types (6 responsibilities)
• Start() has to know what to do for each map type. (3 responsibilities)

*Technically each variable is a responsibility but I have simplified it.

For the final total, what happens if you add a fourth map type? You have to add more state variables (1+ responsibilities), update Load() to be able to clear and initialize state (2 responsibilities), and update Start() to handle the new behavior (1 responsibility). So:

Number of Map responsibilities: 12+

Number of changes required for new map: 4+

There's other problems too. Odds are, several of the map types will have similar state information, so you'll share variables among the states. This makes it more likely that Load() will forget to set or clear a variable, since you might not remember that one map uses _foo for one purpose and another uses it for a different purpose entirely.

It's not easy to test this, either. Suppose you want to write a test for the scenario "When I create a 'spawn monsters' map, the map should spawn one new monster every five seconds." It's easy to discuss how you might test this: create the map, set its type, start it, wait a little bit longer than five seconds, and check the enemy count. However, our interface currently has no "enemy count" property. We could add it, but what if this is the only map that has an enemy count? If we add the property, we'll have a property that's invalid in 2/3 of the cases. It's also not very clear that we are testing the "spawn monsters" map without reading the test's code, since all tests will be testing the Map class.

You could certainly make Map an abstract base class, Start() MustOverride, and derive one new type for each type of map. Now, the responsibility of Load() is somewhere else, because an object can't replace itself with a different instance. You may as well make a factory class for this:

Class MapCreator
    Public Function GetMap(mapType) As Map
End Class

Now our Map hierarchy might look something like this (only one derived map was defined for simplicity):

Public MustInherit Class Map
    Public MustOverride Sub Start()
End Class

Public Class RentalMap
    Inherits Map
    
    Public Overrides Sub Start()
End Class

Load() isn't needed anymore for reasons already discussed. MapType is superfluous on a map because you can check the type of the object to see what it is (unless you have several types of RentalMap, then it becomes useful again.) Start() is overridden in each derived class, so you've moved the responsibilities of state management to individual classes. Let's do another SRP check:

Map base class 0 responsibilities

Map derived class

• Must manage state (1)
• Must perform some type-specific work (1)

Total: 2 responsibilities

Adding a new map (Same as above) 2 responsibilities

Total number of per-class responsibilities: 2

Cost of adding a new map class: 2

This is much better. What about our test scenario? We're in better shape but still not quite right. We can get away with putting a "number of enemies" property on our derived class because each class is separate and we can cast to specific map types if we need specific information. Still, what if you have RentalMapSlow and RentalMapFast? You have to duplicate your tests for each of these classes, since each has different logic. So if you've got 4 tests and 12 different maps, you'll be writing and slightly tweaking 48 tests. How do we fix this?

What did we do when we made the derived classes? We identified the part of the class that was changing each time and pushed it down into sub-classes. What if, instead of subclasses, we created a separate MapBehavior class that we can swap in and out at will? Let's see what this might look like with one derived behavior:

Public Class Map
    Public ReadOnly Property Behavior As MapBehavior
    
    Public Sub SetBehavior(behavior)
    Public Sub Start()
End Class

Public MustInherit Class MapBehavior
    Public MustOverride Sub Start()
End Class

Public Class PlayerSpawnBehavior
    Public Property EnemiesPerSpawn As Integer
    Public Property MaximumNumberOfEnemies As Integer
    Public ReadOnly Property NumberOfEnemies As Integer
    
    Public Sub SpawnEnemy()
    Public Sub Start()
End Class

Now using a map involves giving it a specific MapBehavior and calling Start(), which delegates to the behavior's Start(). All state information is in the behavior object, so the map doesn't really have to know anything about it. Still, what if you want a specific map type, it seems inconvenient to have to create a behavior then create a map, right? So you derive some classes:

Public Class PlayerSpawnMap
    Public Sub New()
        MyBase.New(New PlayerSpawnBehavior())
    End Sub
End Class

That's it, one line of code for a new class. Want a hard player spawn map?

Public Class HardPlayerSpawnMap
    Public Sub New()
        ' Base constructor must be first line so call a function that creates the behavior
        MyBase.New(CreateBehavior()) 
    End Sub
    
    Private Function CreateBehavior() As MapBehavior
        Dim myBehavior As New PlayerSpawnBehavior()
        myBehavior.EnemiesPerSpawn = 10
        myBehavior.MaximumNumberOfEnemies = 300
    End Function
End Class

So, how is this different from having properties on derived classes? From a behavioral standpoint there's not much different. From a testing viewpoint, this is a major breakthrough. PlayerSpawnBehavior has its own set of tests. But since HardPlayerSpawnMap and PlayerSpawnMap both use PlayerSpawnBehavior, then if I've tested PlayerSpawnBehavior I don't have to write any behavior-related tests for a map that uses the behavior! Let's compare test scenarios.

In the "one class with a type parameter" case, if there are 3 difficulty levels for 3 behaviors, and each behavior has 10 tests, you'll be writing 90 tests (not including tests to see if going from each behavior to another works.) In the "derived classes" scenario, you'll have 9 classes that need 10 tests each: 90 tests. In the "behavior class" scenario, you'll write 10 tests for each behavior: 30 tests.

Here's the responsibility tally: Map has 1 responsibility: keep track of a behavior. Behavior has 2 responsibilities: maintain state and perform actions.

Total number of per-class responsibilities: 3

Cost of adding a new map class: 0 (reuse a behavior) or 2 (new behavior)

So, my opinion is that the "behavior class" scenario is no more difficult to write than the "derived classes" scenario, but it can significantly reduce the burden of testing. I've read about techniques like this and dismissed them as "too much trouble" for years and only recently realized their value. This is why I wrote nearly 10,000 characters to explain it and justify it.

Answer 2

You should subclass wherever your child type is some sort of specialization of the parent type. In other words, you should avoid inheritance if you just need functionality. As the Liskov Substitution Principle states: "if S is a subtype of T, then objects of type T in a program may be replaced with objects of type S without altering any of the desirable properties of that program"

Answer 3

In your case i would go with a hybrid approach (this might be called composition, i don't know), where your map mode variable is actually a separate object that stores all related data/behavior to the map's mode. This way you can have as many modes as you like without actually doing too much to the Map class.

gutofb7 nailed it on the head as to when you want to subclass something. Giving a more concrete example: In your Car class, would it matter anywhere in your program what type of car you were dealing with it? now if you subclassed Map, how much code would you have to write that deals with specific subclasses?

Answer 4

In the particular problem you talked about with the maps and spawning, I think this is a case where you want to favour composition over inheritance. When you think about it, they aren't exactly three different types of map. Instead, they are the same map with three different strategies for spawning. So if possible, you should make the spawning function a separate class and have an instance of a spawning class as a member of your map. If all the other differences in "modes" for your maps are similar in nature, you might not have to subclass the map at all, although subclassing the different components (i.e. have a spawn_strategy base class and subclass the three types of spawning from that), or at least giving them a common interface, will probably be necessary.

Given your comment that each type of map is meant to be conceptually different, then I would suggest subclassing, as that seems to fulfill Liskov's substitution principle. However, that is not to say you should give up on composition entirely. For those properties which every type of map has, but may have different behaviour/implementation, you should consider making your base class have them as components. That way you can still mix and match functionality if you need to, while using inheritance to maintain a separation of concerns.