Does deciding about Liskov sub.princple compliance require a contract?

Question

I have read two opinions on the subject. Let's assume the following simple code:

class Enemy
{
    public virtual void CheckHealth()
    {
        if (Life <= 0)
            Dispose();
    }
}

class Boss : Enemy
{
    public override void CheckHealth()
    {
        if (Life <= 0 && PowerSourceDestroyed)
            Dispose();
    }
}

1. From a first look, the LSP is obviously violated as the behavior of the child is not the same as its parent's (if the base class is replaced by the child, it might not work due to the strengthened precondition which is explicitly forbidden by LSP). However, many think that actually to decide whether LSP is violated or not, the contract must be known because the contract might state: "Once the life is <= 0, the system attempts to destroy the enemy". In that case, the LSP would not be violated as the behavior of the child complies.
2. But I am thinking, do I really need the contract? In the end, LSP is formulated as follows: Let O(x) be a property provable about objects x of type T. Then O(y) should be true for objects y of type S where is a subtype of T. I mean, if something is supposed to be provable then I think only the actual representation matters, which in this case seems like the LSP is violated - the property of the parent is not valid for the child because the child imposed an additional condition.

Answer 1

Update

Part of the issue here is that you misunderstand the life<0 as a precondition. RubberDuck explained it better than I could:

@EzoelaVacca I believe you may be misunderstanding the meaning of “precondition”. A precondition is not the internal check you’ve shown in your example. A precondition is what must be true prior to calling a method in order to avoid exceptions/undefined behavior. In other words, a precondition is the state of the system prior to calling a method. It’s that state of the system that can not be made stricter.

Update You dismiss the explanation without actually understanding it:

I do understand that, I included the check so that it is visible what precondition is expected. It does not change the fact that the precondition is stronger in the derived class.

Your continuation proves that you do not understand what RubberDuck meant. The check is not a precondition.

Your example does not have a different precondition between Boss and Enemy. The calling code should not have any expectation of the life value, nor whether it passes the <0 check before calling the method.

The entire point of the method is so that the method can be called without needing to pre-check. The calling code is able to call the method without needing to care about pre-checking if the enemy is dead.
Because if that wasn't the case, then your method should've been a Dispose() without a built-in check.

---

This answer is partly based on your comment to the other answer:

No, the provable property is that when you call the method CheckHealth with life<=0, the function Destroy() is called. This is not true for the child. It is clearly violating the requirement of not strengthened pre-conditions.

You're getting caught up on the actual check (life<=0), not the intention of the method.

First of all, the method is wrongly named. It does not "check health", it tells the enemy to "dispose yourself when you're dead".

Checking the health is indirectly part of it, as it is part of what defines "death" for an enemy. However, as your code shows, the definition for a boss death is different: they need to both have their health dropped to zero and their power source destroyed.

Both for the enemy and the boss, the "dispose yourself when you're dead" contract is the same. The contract merely stipulated a conditional dispose. It does not stipulate the exact condition (what is death?), nor what a dispose actually entails (e.g. a boss may have more resources to dispose of when it dies - which does not violate LSP in any way)

To make this clear, I could rewrite your example to:

class Enemy
{
     public sealed void DisposeIfDead()
     {
         if (this.IsDead)
           Dispose();
     }

     protected virtual bool IsDead => Life <= 0;
}

class Boss: Enemy
{   
     protected override bool IsDead => Life <= 0 && PowerSourceDestroyed;
}

This proves the point that the DisposeIfDead() does not behave differently. It simply relies on a different definition of "death". Omitting other differences, a Boss is different from an Enemy specifically because things about them are different, including death conditions.

---

The LSP focuses on derived classes who behave unexpected from the point of view of a third party.

Let's say I have a method that triggers enemy behaviors. Note that I changed your code example slightly to make the example clearer.

public void HandleEnemyBehavior(Enemy enemy)
{
    bool isDead = enemy.DisposeIfDead();

    if(!isDead)
    {
        enemy.MakeMove();
        enemy.AttackRandomTarget();
    }
}

Let's look back at what you said:

No, the provable property is that when you call the method CheckHealth with life<=0, the function Destroy() is called. This is not true for the child.

Notice how the third party (HandleEnemyBehavior()) does not care how death is defined. It only care that the method gets called, and in this case it cares about knowing if a death occurred.

HandleEnemyBehavior() does not have any expectation as to what defines "death" for an enemy. Therefore, Boss does not break the Enemy contract. As far as the HandleEnemyBehavior() method is concerned, every enemy that can be passed to it (Boss/Enemy) behaves the same way. It does not violate LSP.

I'll give an example of a third enemy type that does violate LSP:

public class Ghost : Enemy
{
    protected override bool IsDead => throw new Exception("Ghosts can't die!");
}

The problem is a bit simplistic (exceptions are clearly not a good idea) but it's a clear example.

This violates LSP, because HandleEnemyBehavior() is now unable to handle any enemy (Boss/Enemy/Ghost) in the exact same way. The only way to get the method to work is:

public void HandleEnemyBehavior(Enemy enemy)
{
    bool isDead = false;

    if( !(enemy is Ghost) )
         isDead = enemy.DisposeIfDead();

    if(!isDead)
    {
        enemy.MakeMove();
        enemy.AttackRandomTarget();
    }
}

And this is the issue that violates LSP. HandleEnemyBehavior() needs to handle ghosts differently from bosses/enemies. Implicitly, HandleEnemyBehavior() needs to be aware of the immortality of the ghost, which violates the idea of encapsulating the definition of death inside the Enemy class (and its subtypes).

Somewhat obviously, the correct implementation of Ghost would be protected override bool IsDead => false;, which does not violate LSP. The exact definitions of death aren't really the focus of the answer, it's more a matter of observing the external behavior of a class and its subtypes.

---

Your expectation of the violation of the LSP suggests that you're thinking of a scenario like this:

public void KillEnemy(Enemy enemy)
{
    enemy.Life = 0;
    enemy.DisposeIfDead();

    //The enemy is now dead.
}    

This would work for an Enemy but not for a Boss (if their power source is still active).

However, you've now violated SRP and encapsulation. The KillEnemy() method has an expectation of the definition of death. That defeats the purpose of encapsulating the definition of death inside the Enemy class.

The issue is in the intention of DisposeIfDead(). It implies that the caller (KillEnemy()) tasks the object (Enemy or a subtype) with deciding if it's dead or not.

SRP comes into play here. The method implies that the object defines death criteria. Therefore, if KillEnemy() were to also contain death criteria, you've written the same thing twice. Both the object and the KillEnemy() method each have their own definition of death, which may or may not be the same. That is a clear violation of SRP.

---

No, the provable property is that when you call the method CheckHealth with life<=0, the function Destroy() is called.

This is the internal logic of the enemy class. Using encapsulation, outside agents are unaware of the inside logic of the enemy class.
LSP is violated when the external handling of the class needs to differentiate between subtypes of the class.

LSP ensures that when a third party handles a type of Enemy, that it does not matter if the real object is an Enemy or any of its subtypes. The third party does not need to care about the subtype of the object.

---

In response to one of your comments:

An example from a book: Base class requires an INT to be < 100. The child requires it to be < 50. LSP is violated.

That is an overly simplistic example. The behavior encountered when you pass 75 to the child class very much influences whether LSP was violated.

If it throws an immediate validation exception, you are correct that it violates LSP.

However, if it merely behaves differently (e.g. returns a different value) but does not misbehave (exception, nonsensical information, data corruption), then LSP is not violated.

"Stricter" is a really bad metric to test for violation. Because while you can argue that the boss has "stricter criteria for death", I could argue that the boss has "simpler criteria for staying alive".
The only difference is in the name of the variables and methods, there is no technical distinction between the two.

---

Answer 2

if something is supposed to be provable then I think only the actual representation matters,

Nope. If you read just below the fragment you quoted from the paper by Liskov and Wing you will find:

A type’s specification determines what properties we can prove about objects.

So it is not the actual representation but the specification what matters.

Since the actual representation satisfies several, different specifications, you need to make the specification (the contract) explicit in order to tell whether the Subtype Requirement (aka LSP) holds.

Answer 3

Precondition is something that the calling code is responsible for, not the callee who actually should rely on the calling code to have met the preconditions. With regards to your code, the derived class imposed more restrictions for Dispose method to be called.

To be LSP compliant (and DbC, where the caller has to make sure preconditions apply), I would suggest a solution like this:

class Enemy

{

public virtual bool CanBeDestroyed()

{

  return life<=0;

}

    public void CheckHealth()

    {       assert.CanBeDestroyed(“true”); //precondition: CanBeDestroyed is true, caller’s responsibility to check before the method is called

            Dispose();

    }

}



class Boss : Enemy

{

public override bool CanBeDestroyed()

{

  return life<=0 && PowerDestroyed;

}

}

Answer 4

If there is no explicitly stated contract, you are basically on your own:

1. As an implementer, you should consider all working code which does use your interface, past, present, and future, as sacrosanct.
   Thus, all that code you might not know, or even ever learn, about constrains what you might consider your contract. Put another way, just about all changes are breaking changes.

2. As a consumer, you have to try divining the future, specifically all the code, past, present, and future, whether you will ever know about it or not, which implements or extends the contract.
   That basically means all you can say is "it exists, and using it might have some effect". Yes, that's a completely unsatisfactory state of affairs.

Luckily, there are conventions, hints in the names, and the principle of least surprise to let you guess at a somewhat more useful contract. The problem is you might guess wrong, and while you might never know it, there might be quite a fallout from that.

---

So, does the derived class violate the LSP?

Strictly yes, practically who knows, the contract was never codified.

Answer 5

The LSP is not violated.

There is nothing really provable about CheckHealth(), (being a void doesn't help) You could argue that "sometimes Dispose is called" but that won't change the result of the Method call and is equally true for Boss.

The LSP is broken when a change in a child class would cause the program to crash when using the child class rather than its parent.

Let's expand you example to:

void CheckHealth(int percentageLife) {}

Now you have an unwritten expectation of percentageLife being between 0 and 100. which could cause issues if broken in a child class.

Lets go further, we would want to add bounds checking

void CheckHealth(int percentageLife) {
    if(percentageLife < 0) {throw new NotAPercentageException();}
    if(percentageLife > 100) {throw new NotAPercentageException();}
    ...
}

So now in c# its arguably the same, because we cant specify exceptions in the compiler defined contract. But in Java we would state that these might be thrown and the compiler would know about it.

Clearly the LSP must be language agnostic. So it must be arguable that the origional

//percentage is a percentage, dont send > 100 or < 0 !! obviously!
void CheckHealth(int percentageLife) {}

Is just as much a 'contract' as

void CheckHealth(SpecialPercentageDataType percentageLife) {}

or similar