SOLID principles for object oriented programming are demonstrated in action here. Whilst coding my FPS game, I observed couple of violations (SO) the second time I modified a section of code. This is the classic example of how context helps in catching such violations. Consider the following hierarchical class arrangement
class SUNOVATECHZOMBIEKILL_API ASTPickupInventory : public ASTPickup{
public:
virtual void GiveTo(Pawn* Target) override;
...
}
class SUNOVATECHZOMBIEKILL_API ASTPickupWeapon : public ASTPickupInventory
{
...
}
and finally implementation of GiveTo() function like so
void ASTPickupInventory::GiveTo(Pawn* Target)
{
if(Target == nullptr)
{
return;
}
Super::GiveTo(Target);
ASTInventory* Existing = Target->FindInventoryType(InventoryType, true);
if (Existing == nullptr)
{
ASTInventory* Inv = nullptr;
// Spawn the inventory type class object and attach to pawn
UWorld* const World = GetWorld();
if (World != nullptr)
{
...
}
else
{
UE_LOG(LogSunovatechZombieKill, Log, TEXT("No world found, can't spawn actor of class %s"), *InventoryType->GetName());
}
// Add inventory to target and equip
if(Target->SwitchToRecentPickup())
{
Target->AddInventory(Inv, true);
}
else // or not
{
Target->AddInventory(Inv, false);
}
// This looks out of place
ASTWeapon* MyWeapon = Cast<ASTWeapon>(Inv);
// Generate circular doubly linked list for switching weapons by rotation
if(MyWeapon)
{
Target->GenerateWeaponCDL();
}
}
else // ok this pickup already exists in inventory
{
// This looks out of place as well
// if weapon, increase the ammo
ASTWeapon* MyWeapon = Cast<ASTWeapon>(Existing);
if(MyWeapon)
{
MyWeapon->AddAmmo(10); // to do: move this code to STPickupWeapon maybe
}
}
}
The “out of place” code may require data member (or member function) of ASTPickupWeapon class, for instance AddAmmo() may require amount belonging to the class.
The SRP (Single Responsibility Principle) says that class should have only one function, here, involving general inventory only. Specializing to ASTWeapon seems responsibility of ASTPickupWeapon. Open/Close principle says that class should be open for extension and closed for modification which also seems like a violation here. This implies violation when weapon specific modifications are applied. Hence if SRP is violated, so is Open/Close principle.
A simple solution is to write overridable methods like so
class SUNOVATECHZOMBIEKILL_API ASTPickupInventory : public ASTPickup{
protected:
/**
* @brief Function for dealing with non-existant and post inventory spawn procedures
*/
virtual void DealWithNonExistentInventory(ASTInventory* NonExistingInventory, Pawn* Target);
/**
* @brief Function for dealing with existing inventory procedure
*/
virtual void DealWithExistingInventory(ASTInventory* ExistingInventory, Pawn* Target);
...
}
by doing this, we have opened passage way for child class ASTPickupWeapon to do ASTWeapon specific computing.
void ASTPickupInventory::GiveTo(Pawn* Target)
{
if(Target == nullptr)
{
return;
}
Super::GiveTo(Target);
ASTInventory* Existing = Target->FindInventoryType(InventoryType, true);
if (Existing == nullptr)
{
ASTInventory* Inv = nullptr;
// Spawn the inventory type class object and attach to pawn
UWorld* const World = GetWorld();
if (World != nullptr)
{
...
}
else
{
UE_LOG(LogSunovatechZombieKill, Log, TEXT("No world found, can't spawn actor of class %s"), *InventoryType->GetName());
}
// Add inventory to target and equip
if(Target->SwitchToRecentPickup())
{
Target->AddInventory(Inv, true);
}
else // or not
{
Target->AddInventory(Inv, false);
}
DealWithNonExistentInventory(Inv, Target);
}
else // ok this pickup already exists in inventory
{
DealWithExistingInventory(Existing, nullptr);
}
}