Does it violate SRP to reject subclass method parameter

Question

I was having a good read on Eric Lippert's blog about Wizard and Warriors.

It suggests the creation of a Rules class, quote:

We keep on talking about “rules”, and so apparently the business domain of this program includes something called a “rule”, and those rules interact with all the other objects in the business domain. So then should “rule” be a class? I don’t see why not!

The rules:

• A warrior can only use a sword.
• A wizard can only use a staff.

Maybe I'm not thinking about it in the right way, but suppose I have the following GameRules class:

public final class GameRules {

    public static boolean verifyifwizardcancarry(Weapon weapon){
        boolean canCarry  = false
        if weapon is a a staff set canCarry to true
        return canCarry;
    }
}

and Player:

public abstract class Player{   

   private List<Weapon> weapons;    
   public abstract void add(Weapon weapon);

}


public final class Wizard extends Player{

   @Override 
   public void add(Weapon weapon){

      if(GameRules.verifyifwizardcancarry(weapon){
          // - code to add weapon to inventory
      }
    }
 }

Does rejecting a weapon base on type (regardless of where that logic is placed) violate LSP?

In my add(Weapon weapon) method I'm promising that I will accept a Weapon of any kind, so to reject it based on type is a violation of LSP, correct? If so, how would I enforce the above rules?

Answer 1

If a subclass violates the LSP can only be verified in context of the contract of a method. Some programming languages provide explicit language support for constracts, but even if not, one can always describe the contract in the documentation.

So lets say the abstract add method looks like this:

 // - may or may not add weapon to the list "weapons"
 // - if adding is not possible, an exception can be thrown
 public abstract void add(Weapon weapon);

as long as a subclass implements add obeying these rules, their is no LSP violation. However, if the contract reads

 // - adds weapon to the list "weapons"
 // - no exception will be thrown
 public abstract void add(Weapon weapon);

the implementation in your example will violate the LSP.

In case someone did not specify any behaviour, add has no "provable properties", so there is nothing to violate in terms of the LSP, at least not formally.

However, naming of the method, its parameters and choice of return type can implicitly lead to a certain expectation for the user of this interface, which can be interpreted as some kind of "informal" contract. For example, by naming the method TryAdd instead of just add, it would be much clearer that it does not guarantee to add the weapon to the list. By giving it a boolean return type, this would probably make the user expect the method not to throw an exception in case the weapon is not added. If a subclass then behaves differently, this can be seen as an "informal" violation of the LSP, or just a violation of the POLA.

Answer 2

'Wizards and Warriors' is really a terrible example to choose. It opens the field to all sorts of solutions that don't really help explain the issue.

I much prefer Cats and Dogs

public abstract class Animal
{
    public abstract List<Animal> Children { get; set; }
}

public class Cat : Animal
{
    public override List<Cat> Children { get; set; }
}

public class Dog : Animal
{
    public override List<Dog> Children { get; set; }
}

This doesn't work even though it seems like it should. If you try:

public class Cat : Animal
{
    public override List<Animal> Children { get; set; }
}

then you can get

Cat.Children.Add(new Dog);

and you are back in the Wizards with Swords area. But while its conceivable for a wizard to hold a sword, no-one is going to think a cat can have puppies. That would be insane(1)

The very nature of inheritance is to allow the substitution that we are trying to prevent. You can throw exceptions, or just not add but then you break the LSP

The solution (well one of the solutions) is Generics

class Animal<T> where T : Animal<T>
{
    List<T> Childern;
}

class Cat : Animal<Cat> {}

Of course some classes can be an Ass and require more complex solutions.

(1) https://youtu.be/nC9GXlYb-ek