Complexity
A code base grows over the course of its lifetime; features are added, redesigned, and expanded upon. It is good to put some thought into the way your code changes before you find yourself lost in a file that has grown to be thousands of lines long, and too complex or dependent to refactor.
Ironically there are a lot of factors that contribute to the complexity of code bases, on an architectural level, all the way down to the individual functions. In this section we will focus on the lower level considerations.
Purpose
In order to reduce complexity at the unit level (classes, methods), try to think about what purpose the unit serves. Is its purpose clear? And does it serve JUST its purpose, or does it try to do more than it should?
We have discussed expressing intent in 📄 Naming. And indeed, proper naming is important to communicate the purpose of a piece of code, and thus help developers understand what sort of code belongs in a certain unit, and what code should be separated and moved outside of a unit.
A danger that is not yet discussed is when code does more than it promises. Code that does too much may lead to numerous issues. To name a few, it may; cause unclarity about what a certain unit actually does; it may compromise a unit's comprehensiveness; and it may make it more difficult to find where in the code base a certain piece of functionality is situated.
​public class Weapon
{
private readonly IMessageService _messageService;
private readonly Damage _damage;
public void DealDamage(ICreature target)
{
if (target.CreatureType == CreatureType.SomeKindOfFireCreature)
{
if (_damage.Type == DamageType.Water)
{
_damage.Amount *= 2;
}
else if (...)
{
// etc.
}
}
else if (...)
{
// etc.
}
if (target.IsVulnerable)
{
target.Health -= _damage.Amount;
}
else
{
_messageService.ShowMessage("This creature cannot be damaged!");
}
}
}
Oops! That's an awful lot of code for what you would expect to essentially be little more than target.Health -= damage.Amount;
So what are the issues? Beyond its simple purpose of subtracting one number from another, it does the following;
- Check whether the enemy is vulnerable for a specific damage type (not its responsibility)
- And subsequently applies a damage multiplier (side effect)
- Checks whether the target is vulnerable (not its responsibility)
- And sends a message (still not its responsibility)
- Subtracts health from the target (also a side effect, though perhaps more expected)
Side effects
Let's first talk about side effects. If you think about functions/methods in a puristic way, they very simply take an input (or not) and produce an output.
For example this following Add
​ method, which takes two integers as its input, and returns one integer as its output:
int Add(int a, int b)
{
return a + b;
}
This method is pure; it only operates on its inputs and produces its promised output. It does not do anything more than it has promised.
Now compare it to the following example:
public class Calculator
{
public int Result { get; set; }
public void Add(int a, int b)
{
Result = a + b;
}
}
Instead of returning the sum of the two integers, the result is written to a variable outside of the method's scope. This divergence from the simple input -> function -> output flow is what is causing a side effect. Namely, a value outside of the method's scope (Result
) gets updated.
Side effects contribute to complexity in the sense that the scope of a certain unit of code (whether that is a class or a method) becomes bigger. Instead of just knowing what happens inside of the given unit, you now also have to consider the state of the associated code that gets modified because of the side effect.
Multiple responsibilities
Another prevalent issue in the above example is the various tasks that this supposedly simple method ends up doing. Its definition DealDamage(Target)
would suggest that it simply subtracts the damage associated with the weapon from the given target's health. Yet upon closer inspection the method also calculates damage modifiers, checks invulnerability and sends messages about this state. As the example insinuates, all these responsibilities can make the code for executing this simple task waaaaaaaaaaaay bigger than it has to be.
Let's consider an alternative approach:
public interface IDamageable
{
int Health { get; }
bool IsVulnerable { get; }
void ApplyDamage(Damage damage);
}
public class CrazyFireEnemyThing : ICreature, IDamageable
{
// CreaureType from the ICreature interface.
public CreatureType CreatureType { get; } = CreatureType.SomeKindOfFireCreature;
// Health and IsVulnerable from the IDamageable interface.
public int Health { get; } = 100;
public bool IsVulnerable { get; } = true;
public void ApplyDamage(Damage damage)
{
if (!IsVulnerable)
{
return;
}
// Creating a local copy of damage.Amount prevents side effects
// as we don't need to modify the Damage object.
var damageAmount = damage.Amount;
// Since we already know that this CrazyFireEnemyThing is a Fire type
// we only need to check the damage types that this enemy is vulnerable to.
if (damage.Type == DamageType.Water)
{
​ damage *= 2;
}
Health -= damage;
}
}
By placing the responsibility of checking whether the enemy is vulnerable to a certain type of damage inside of its own ApplyDamage
​ method, we already know what CreatureType
​ we are dealing with, and won't have to check every single possibility.
And finally our Weapon
class simply passes its own Damage object to an IDamageable interface:
public class Weapon
{
private readonly Damage _damage;
public void DealDamage(IDamageable target)
{
target.ApplyDamage(_damage);
}
}
This change in our code has made our code easier to reason with as the responsibilities have a more logical separation. Additionally our code is now easier to extend as we can very easily add a new type of target without ever having to update our Weapon
class.