Overview
This is the second post from the design pattern series. In this post we will apply the steps you should make before using a design pattern (applying clean coding and SOLID principles, refactoring). This steps were presented in my previous post.
We will identify the problems that our legacy code has and try to solve them.
Scenario
After the initial creator of a tower defense game left the company, we are responsible to develop and maintain it.
At this moment the game has 3 unit types
- Money (generates money to buy new units)
- Offense (shoots bullets, arrows,etc at the enemy)
- Delay (slows down the enemy).
We received the following requirements:
- add a new tower unit type that that will bewitch enemies and make them attack one another for 1sec.
- make the code easy to extend (multiple units will be added in the future).
- for each unit type there can exist several implementations (ex: we can have 5 offensive unit types).
Code Overview
We will take a look at the current implementation.
The complete code can be found here.
Unit represents the interface that has to be implemented by each unit. It:
- defines an enum value for each unit type.
- has a method added for each action that can be performed by a unit
class Unit
{
public:
enum class Type
{
TypeGun,
TypeMoney,
TypeSlower
};
virtual Type getType() = 0;
virtual void shoot() = 0;
virtual void generateMoney() = 0;
virtual void freezeEnenmy() = 0;
};
Money class represents the implementation of the money generator tower unit. It only provides an implementation for the generateMoney method, leaving the other methods empty.
class Money : public Unit
{
public:
Money() = default;
virtual void shoot(){}
virtual void generateMoney()
{
std::cout << "generating money" << std::endl;
}
virtual void freezeEnenmy(){}
virtual Type getType()
{
return Type::TypeMoney;
}
};
Delay class represents the implementation for the delay unit.
class Delay : public Unit
{
public:
Delay() = default;
virtual void shoot(){}
virtual void generateMoney(){}
virtual void freezeEnenmy()
{
std::cout << "slowing down" << std::endl;
}
virtual Type getType()
{
return Type::TypeSlower;
}
};
Offensiveclass represents the implementation for the offensive unit.
class Offensive : public Unit
{
public:
Offensive() = default;
virtual void shoot()
{
std::cout << "shooting" << std::endl;
}
virtual void generateMoney(){}
virtual void freezeEnenmy(){}
virtual Type getType()
{
return Type::TypeGun;
}
};
Offensive and Delay classes have the same structure as the Money class, therefore they provide a real implementation for one of the interface methods and let the other methods empty.
Usage overview
In the main method an instance for each unit is created and passed to the startUnit function.
startUnit function contains a switch case and for each unit type calls the specific method of that unit. Example for a money type unit it will call generateMoney.
void startUnit(Unit*);
int main()
{
Unit* offensive = new Offensive();
Unit* delay = new Delay();
Unit* money = new Money();
startUnit(offensive);
startUnit(delay);
startUnit(money);
getchar();
return 0;
}
// Function used by the map to start the tower's ability
void startUnit(Unit* unit)
{
switch (unit->getType())
{
case Unit::Type::TypeGun:
unit->shoot();
break;
case Unit::Type::TypeMoney:
unit->generateMoney();
break;
case Unit::Type::TypeSlower:
unit->freezeEnenmy();
break;
default:
std::cout << "Invalid Type" << std::endl;
}
The code output is presented bellow:
shooting
slowing down
generating money
Class diagram
The above diagram shows us the relationship between the current classes in our code:
- Type enum is nested in Unit interface.
- Offensive, Money and Delay classes are implementations of Unit interface.
Improving the code
Lets see how can we improve the current code.
For the first step I propose we make the code cleaner and safer with the following changes:
- give classes and enums more suggestive names(examples: Offensive, Delay – sound IMO too general)
- fix memory leaks( we use the new keyword 3 times but never use delete in the main method)
- handle invalid objects in the startUnit method(if we pass an invalid object it will crash).
For the second step lets check if we respect all SOLID principles.
Note: For more information on SOLID check here. In the future I will also write an article on this topic.
The code does not respect the “I” from SOLID (Interface segregation principle) and the “O” (open-close principle)
Interface segregation principle
According to Wikipedia the interface segregation principle states:
Many client-specific interfaces are better than one general-purpose interface
Each unit(Offensive, Delay and Money) does one action but has to implement all the methods from the interface(Unit).
The principle states that is better to have more specific interfaces (define an interface for each unit type) than have one interface that has to be inherited by each unit (even if most of the methods will not contain implementation).
Open close principle
According to Wikipedia the open closed principle states:
Software entities … should be open for extension, but closed for modification.
In a simple way this rule refers to what should happen wen we extend our existing functionality.
Closed for modification states that we shouldn’t modify the existing code when we add new functionality. In our case we:
- add another method to the Unit interface
- add the same method to each child (Money, Delay, Offensive).
Open for extension states that the new functionality should extend current classes/interfaces. For example depend only on the Unit interface (or other interfaces, classes, etc) without affecting the existing code.
Note: before starting to refactor the existing code you should write unit test to make sure that the code behavior will remain unchanged
Results
Let’s see how the new classes look after some refactoring
Changes in the Unit interface:
- renamed to IUnit (for this post I will use the java notation for simplicity)
- has one method that returns the current type.
- the values from the enum Type are renamed:
- TypeGun – Offensive
- TypeMoney – MoneyGenerator
- TypeSlover – Delay
class IUnit
{
public:
enum class Type
{
Delay,
Offensive,
MoneyGenerator
};
virtual Type getType() = 0;
};
I introduced 3 new interfaces:
- IOffensive – specialized by all classes that will attack the enemy
- IDelay – specialized by all classes that want to implement a delay type tower unit.
- IMoneyGenerator – specialized by all classes that will be a money generator tower unit.
The Interfaces are presented bellow.
class IDelayTower : public IUnit
{
public:
virtual void delayTroll() = 0;
};
class IMoneyGeneratorTower : public IUnit
{
public:
virtual void generateMoney() = 0;
};
class IOffesniveTower : public IUnit
{
public:
virtual void shootTroll() = 0;
};
The implementations for the 3 interfaces are represented by: OffensiveTower, DelayTower and MoneyGeneratorTower.
At first sight we can say that this is an overhead, instead of having 1 interface we have 4.
This approach provides a benefit in the case that we:
- have multiple implementations for same tower type ( example 3 implementations for offensive tower).
- they will be multiple changes in the code.
main method has a similar structure. We only took care to free the memory.
int main()
{
IUnit* offensive = new OffesniveTower();
IUnit* delay = new MoneyGeneratorTower();
IUnit* money = new DelayTower();
startUnit(offensive);
startUnit(delay);
startUnit(money);
delete offensive;
delete delay;
delete money;
getchar();
return 0;
}
In startUnit there appeared a static_cast for each unit type, caused by Unit interface because it does not have information about each unit type operation.
void startUnit(IUnit* unit)
{
if (unit == nullptr)
{
std::cout << "Invalid Tower" << std::endl;
return;
}
switch (unit->getType())
{
case IUnit::Type::Delay:
static_cast<IDelayTower*>(unit)->delayTroll();
break;
case IUnit::Type::MoneyGenerator:
static_cast<IMoneyGeneratorTower*>(unit)->generateMoney();
break;
case IUnit::Type::Offensive:
static_cast<IOffesniveTower*>(unit)->shootTroll();
break;
default:
std::cout << "Invalid Type" << std::endl;
}
The class diagram for our new structure is the following:
Adding new functionality before and after.
To add our new code before refactoring we have to do the following:
- modify the interface by adding a burning method
- add a value to the enum Type for the burning value
- update all existing implementations of the Unit interface with the new method
- add a new case to the switch from from startUnit method
Now we have the following steps:
- add an interface for the burning unit type
- add a value to the enum Type for the burning value
- add a new case to the switch from from startUnit method
Remaining problem
We now respect the “I” from SOLID – we use specific interfaces for each unit type.
We partially respect “O” from SOLID – we don’t modify the existing Unit interface , we extend it.
The problem that remains is the dependency on specific operations.The dependency of the startUnit method on each different unit operations. For each different type you have to add a new case to the switch and know the method type.
We have a different method for each operation and the client in our case the startUnit function has to know about each functions so we should find a way to make startUnit function independent of the request. Refactor
Now lets think what would happen if we have to add 1000 units or even more: a nightmare.
In the next post we will look for a pattern that solves our problem and apply it.
1 thought on “Introduction to design patterns PART 2”