1

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?

  • Wizards don't extend Player - you're encoding rules implicitly in the type, rather than explicitly in actual running code. – Telastyn Jun 5 '18 at 17:54
  • @Telastyn - I'm still learning here, but could you provide an example of what you're saying? Wizard in my example does extend Player – user306112 Jun 5 '18 at 17:57
  • 2
    You need not promise that add() will always add. For example, java.util.Collection.add() might return false if the item were already there, or throw an IllegalWeaponException. Or, I don't see why the Wizard can't carry a sword, just don't allow him to use it later. – user949300 Jun 5 '18 at 18:21
  • @user949300 - So throwing an exception is not violation of LSP? I'm only following the rules stated in the blog, so I don't think I can prevent the Wizard from using it if the character has it, but it might beg the question of why I allowed the character to pick it up if they couldn't use it. Short answer, don't allow them to pick it up. The only way I can think of is to throw an exception based on type or return false. The GameRules class makes sense, because as stated in blog, rules can enforce other rules or depend on each other. It makes sense to keep the rules in place. – user306112 Jun 5 '18 at 18:26
  • 1
    The Wizards and Warriors post is actually a series of posts. Did you read all of them? – Robert Harvey Jun 5 '18 at 20:02
3

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.

  • Yes, it's all a matter of the contract. Though I wouldn't say that if a contract isn't properly specified, it cannot be violated. No, instead all the existing code-base implementing and / or using it becomes that implied contract, whether you know the code, or even about the existence of that code, or not. Such an implied contract is just about impossible to verify or extend though, and thus work with. – Deduplicator Jun 5 '18 at 20:15
  • @Deduplicator: of course, there is more than just sticking formally to the LSP for writing error-free code. And sure, in reality, using code could make assumptions about the unspecified behaviour and it could also be implemented not as robust as it should be, which may create an implicit de-facto contract. But that's reality, not LSP in theory. – Doc Brown Jun 5 '18 at 20:49
  • @DocBrown - Would it be better to have tryandadd(Weapon weapon) as the method name? At least this way, the reader knows there is a possibility that adding might fail, in addition to the exceptions that are possible in the method signature. – user306112 Jun 5 '18 at 23:12
  • Good answer, but might want to mention it may be possible to prevent the exception by preventing the object from being added at compile time using the type system. – Frank Hileman Jun 5 '18 at 23:51
  • @Sveta: yes, I think so (actually I would call it TryAdd). Returning a boolean or some error object to show the success would give the caller the expection that function may not add anything, but won't throw an exception. This is the "principle of least surprise", which is also a way of describing a contract in an informal way. – Doc Brown Jun 6 '18 at 5:49
0

'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

  • Is throwing an exception from the subclass a violation of LSP? The answer above, states as long as it's specified in the method signature in the abstract class, it's not a violation. I'm not sure about C#, and how you would specify that a method throws a specific exception. – user306112 Jun 5 '18 at 23:16
  • yeah, unless you say it might throw. softwareengineering.stackexchange.com/questions/370580 – Ewan Jun 5 '18 at 23:19
  • If I follow the example in the link, the Character and Weapon would have an enum type, and the GamesRules will check what character type is attempting to add what weapon type and if the character can't wield it, it will return false, correct? – user306112 Jun 5 '18 at 23:25
  • In the other example, the EatenFood and Food are essentially the rules in that they determine which Animal can eat it. correct? – user306112 Jun 5 '18 at 23:28
  • That's why its a bad example. Of course you can redesign your classes to avoid the problem, as i do in that link, but it doesn't help demonstrate the problem – Ewan Jun 5 '18 at 23:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy