All credits to AtmaWeapon of http://www.xtremevbtalk.com answering in this thread
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.