Code Contracts and Inheritance(Precondition on overridden method)

Question

Currently code contracts do not allow preconditions on members in derived classes where the member already has a precondition set in the base class(I actually currently get a warning and not an error). I do not understand the logic behind this. I understand that it has to do with Liskov's substitution rule stating that a derived class should always be able to be used in places where the parent is expected. Of course "used" means work as expected. This seems okay to me for interfaces as the different types implementing an interface don't add state and hence can oblige the contract exactly. However, when you inherit from a base class, you are doing so to add state and special functionality and more often than not the overriding method would have extra requirements. Why can't preconditions be ANDed together just like post conditions and object invariants?

Take a look below:

class Speaker
{
    public bool IsPlugged { get; set; }
    protected virtual void Beep()
    {
        Contract.Requires(IsPlugged);
        Console.WriteLine("Beep");
    }
}

class WirelessSpeaker : Speaker
{
    public bool TransmitterIsOn { get; set; }
    protected override void Beep()
    {
        Contract.Requires(TransmitterIsOn);
        base.Beep();
    }
}

You may argue that this class hierarchy breaks Liskov's rule because the wireless speaker may not be able to beep when passed to methods that expect a Speaker. But isn't that why we use code contracts? to make sure that the requirements are met?

Answer 1

Code contracts are not about the meeting of the requirements, but their communication. Callers of Speaker.Beep are bound by a contract that only takes effect in some cases.

WirelessSpeaker narrows the functional space of Speaker - that's where Liskov comes into play. I can only use that particular Speaker effectively if I know it's wireless. In that case, I should explicitly accept WirelessSpeaker, not Speaker, and avoid the substitution issues.

Edit in response to comments:

The author of WirelessSpeaker chooses how to interpret the Beep command. Choosing a new contract, visible at this level but not at the base level, imposes constraints that apply <100% of the time when using Speakers.

If it were to simply not beep when the transmitter isn't on, we wouldn't be talking about Code Contracts. Their intention is not to communicate at runtime, but at design time, the semantics of the call (not just its syntax).

The fact that an exception occurs at runtime, ultimately preventing the "incorrect" call, is largely irrelevant here.

Answer 2

@BryanWatts is right. The classes presented by the OP violate the Liskov Substitution Principle. And you shouldn't use exceptions to control program flow—that's a code smell too. Exceptions are meant for, well, exceptions—exceptional conditions that will not allow your object to behave in an expected manner which could lead to corruption of your object's state and/or future behavior.

You need to ensure that you understand the totality of the Liskov Substitution Principle (LSP). LSP is not about ensuring that interfaces can be used interchangeably.

When an object inherits from another object, it's inheriting all of it's parent's behavior. True, you can override that behavior, but you must be careful in doing so. Let's use your example of a Speaker and a WirelessSpeaker and see how it all falls apart.

public class Speaker
{
    public bool IsPlugged { get; set; }

    public virtual void Beep()
    {
        if (!IsPlugged)
        {
            throw
            new InvalidOperationException("Speaker is not plugged in!");
        }

        Console.WriteLine("Beep.");
    }
}

public class WirelessSpeaker : Speaker
{
    public bool TransmitterIsOn { get; set }

    public override void Beep()
    {
        if (!TransmitterIsOn)
        {
            throw
            new InvalidOperationException("Wireless Speaker transmitter is not on!");
        }

        Console.WriteLine("Beep.");
    }
}

public class IBeepSpeakers
{
    private readonly Speaker _speaker;

    public IBeepSpeakers(Speaker speaker)
    {
        Contract.Requires(speaker != null);
        Contract.Ensures(_speaker != null && _speaker == speaker);
        _speaker = speaker;

        // Since we know we act on speakers, and since we know
        // a speaker needs to be plugged in to beep it, make sure
        // the speaker is plugged in.
        _speaker.IsPlugged = true;
    }

    public void BeepTheSpeaker()
    {
        _speaker.Beep();
    }
}

public static class MySpeakerConsoleApp
{
    public static void Main(string[] args)
    {
        BeepWiredSpeaker();

        try
        {
            BeepWirelessSpeaker_Version1();
        }
        catch (InvalidOperationException e)
        {
            Console.WriteLine($"ERROR: e.Message");
        }

        BeepWirelessSpeaker_Version2();
    }

    // We pass in an actual speaker object.
    // This method works as expected.
    public static BeepWiredSpeaker()
    {
        Speaker s = new Speaker();
        IBeepSpeakers wiredSpeakerBeeper = new IBeepSpeakers(s);
        wiredSpeakerBeeper.BeepTheSpeaker();
    }

    public static BeepWirelessSpeaker_Version1()
    {
        // This is a valid assignment.
        Speaker s = new WirelessSpeaker();

        IBeepSpeakers wirelessSpeakerBeeper = new IBeepSpeakers(s);

        // This call will fail!
        // In WirelessSpeaker, we _OVERRODE_ the Beep method to check
        // that TransmitterIsOn is true. But, IBeepSpeakers doesn't
        // know anything _specifically_ about WirelessSpeaker speakers,
        // so it can't set this property!
        // Therefore, an InvalidOperationException will be  thrown.
        wirelessSpeakerBeeper.BeepTheSpeaker();
    }

    public static BeepWirelessSpeaker_Version2()
    {
        Speaker s = new WirelessSpeaker();
        // I'm using a cast, to show here that IBeepSpeakers is really
        // operating on a Speaker object. But, this is one way we can
        // make IBeepSpeakers work, even though it thinks it's dealing
        // only with Speaker objects.
        //
        // Since we set TransmitterIsOn to true, the overridden
        // Beep method will now execute correctly.
        //
        // But, it should be clear that IBeepSpeakers cannot act on both
        // Speakers and WirelessSpeakers in _exactly_ the same way and
        // have confidence that an exception will not be thrown.
        ((WirelessSpeaker)s).TransmitterIsOn = true;

        IBeepSpeakers wirelessSpeakerBeeper = new IBeepSpeaker(s);

        // Beep the speaker. This will work because TransmitterIsOn is true.
        wirelessSpeakerBeeper.BeepTheSpeaker();
}

This is how your code broke the Liskov Substitution Principle (LSP). As Robert & Micah Martin astutely point out in Agile Principles, Patterns and Practices in C# on pp. 142-143:

LSP makes it clear that in OOD, the IS-A relationship pertains to behavior that can be reasonably assumed and that clients depend on....[W]hen using an object through its base class interface, the user knows only the preconditions and postconditions of the base class. Thus, derived objects must not expect such users to obey preconditions that are stronger than those required by the base class. That is, users must accept anything that the base class could accept. Also, derived classes must conform to all the postconditions of the base [class].

By essentially having the precondition TransmitterIsOn == true for the Beep method of the WirelessSpeaker, you created a stronger precondition than what existed on the base Speaker class. For WirelessSpeakers, both IsPlugged and TransmitterIsOn must be true in order for Beep to behave as expected (when viewed from the perspective of a Speaker), even though a Speaker in and of itself has no notion of TransmitterIsOn.

Also, you violated another SOLID principle, the Interface Segregation Principle (ISP):

Clients should not be forced to depend on methods they do not use.

In this case, a WirelessSpeaker does not need to be plugged in. (I'm assuming we're talking about the audio input connection here, as opposed to an electrical connection.) Therefore, the WirelessSpeaker should not have any property called IsPlugged, yet, because it inherits from Speaker, it does! This is an indication that your object model does not line up with how you intend to use your objects. Again, notice that most of this discussion is centered around the behavior of your objects, and not their relationship to one another.

Moreover, the violation of both LSP and ISP both signal that there's probably been a violation of the Open/Closed Principle (OCP), too:

Software entities (classes, modules, functions, etc.) should be open for extension but closed for modification.

So, at this point it should be clear, now, that we do not use Code Contracts only to ensure that certain preconditions are met when calling methods on objects. No, instead Code Contracts are used to state guarantees (hence the word contract) about the behavior and state of your objects and their methods based on the stated pre- and post-conditions, as well as any invariants you may also have defined.

So, for your speaker class, what you're saying is: If the speaker is plugged in, then the speaker can beep. Ok, so far, so good; that's simple enough. Now, what about the WirelessSpeaker class?

Well, WirelessSpeaker inherits from Speaker. Therefore, WirelessSpeaker also has an IsPlugged Boolean property. Furthermore, because it inherits from Speaker, then in order for the WirelessSpeaker to beep, it must also have its IsPlugged property set to true. "But wait!" you say, "I have overridden the implementation of Beep such that WirelessSpeaker's transmitter must be on." Yes, this is true. But it also must be plugged in! WirelessSpeaker not only inherits the Beep method, but also the behavior of its parent class's implementation! (Consider when a base class reference is used in place of the derived class.) Since the parent class can be "plugged in", so too, can WirelessSpeaker; I doubt this is what you intended when you originally thought of this object hierarchy.

So, how would you fix this? Well, you need to come up with a model better aligned to the behaviors of the objects in question. What do we know about these objects, and their behavior?

1. They're both a kind of speaker.
   • So potentially, a wireless speaker could be a specialization of a speaker. Conversely, a speaker may be a generalization of a wireless speaker.
   • In the current object model (as much as you've posted), there is not much behavior or state that is shared between these two objects.
   • Since there is not much common state or behavior between the two objects, one could argue there shouldn't be an inheritance hierarchy here. I'm going to play devil's advocate with you and maintain the inhertiance hierarchy.
2. They both beep.
   • However, the conditions under which each type of speaker beeps are different.
   • These speakers, therefore, cannot directly inherit one from the other, otherwise, they would share behavior which may not be appropriate to them (and in this case, the existing "shared behavior" is definitely not appropriate for all types of speakers). This resolves the ISP problem.

Ok, so the one piece of shared behavior these speakers have is they beep. So, let's abstract that behavior out into an abstract base class:

// NOTE: I would prefer to simply call this Speaker, and call
// Speaker 'WiredSpeaker' instead--but to leave your concrete class
// names as they were in your original code, I've chosen to call this
// SpeakerBase.
public abstract class SpeakerBase
{
    protected SpeakerBase() { }

    public void Beep()
    {
        if (CanBeep())
        {
            Console.WriteLine("Beep.");
        }
    }

    public abstract bool CanBeep();
}

Great! Now we have an abstract base class that represents speakers. This abstract class will allow a speaker to beep, if and only if the CanBeep() method returns true. And this method is abstract, so any class inheriting this class must provide their own logic for this method. By creating this abstract base class, we have enabled any class that has a dependency on the SpeakerBase class to emit a beep from the speaker if and only if CanBeep() returns true. This also resolves the LSP violation! Anywhere a SpeakerBase can be used and called upon to beep, either a Speaker or a WirelessSpeaker can be substituted and we can be sure of the behavior: if the speaker can beep, it will.

Now all that's left is to derive each of our speaker types from SpeakerBase:

public class Speaker : SpeakerBase
{
    public bool IsPlugged { get; set; }

    public override bool CanBeep() => IsPlugged;
}

public class WirelessSpeaker : SpeakerBase
{
    public bool IsTransmiterOn { get; set; }

    public override bool CanBeep() => IsTransmitterOn;
}

So, now we have a Speaker that can only beep when it's plugged in. We also have a WirelessSpeaker that can only beep if it's transmitter is turned on. In addition, WirelessSpeakers know nothing about being "plugged in". It's simply not a part of their essence.

Furthermore, following the Dependency Inversion Principle (DIP):

1. High-level modules should not depend on low-level modules. Both should depend on abstractions.
2. Abstractions should not depend upon details. Details should depend upon abstractions.

What this means is that consumers of speakers should not depend directly on either Speaker or WirelessSpeaker, but should depend on SpeakerBase instead. This way, no matter what kind of speaker comes along, if it inherits from SpeakerBase, we know that we can beep the speaker if the conditions warrant for the sub-type of speaker referenced by the abstract type in the dependent class. This also means that IBeepSpeakers no longer knows how to put a speaker in the state such that it can beep, as there is no common behavior among speaker types that IBeepSpeakers could use to make such a determination. So that behavior must be passed in as a dependency to IBeepSpeakers. (This is an optional dependency; you could just let the class take in a SpeakerBase and call Beep(), and, if the SpeakerBase object is in the correct state, it'll beep, otherwise it won't.)

public class IBeepSpeakers
{
    private readonly SpeakerBase _speaker;
    private readonly Action<SpeakerBase> _enableBeeping;

    public IBeepSpeakers(SpeakerBase speaker, Action<SpeakerBase> enableBeeping)
    {
        Contract.Requires(speaker != null);
        Contract.Requires(enableBeeping != null);
        Contract.Ensures(
            _speaker != null && 
            _speaker == speaker);
        Contract.Ensures(
            _enableBeeping != null && 
            _enableBeeping == enableBeeping);

        _speaker = speaker;
        _enableBeeping = enableBeeping;
    }

    public void BeepTheSpeaker()
    {
        if (!_speaker.CanBeep())
        {
           _enableBeeping(_speaker);
        }
        _speaker.Beep();
    }
}

public static class MySpeakerConsoleApp
{
    public static void Main(string[] args)
    {
        BeepWiredSpeaker();

        // No more try...catch needed. This can't possibly fail!
        BeepWirelessSpeaker();
    }

    public static BeepWiredSpeaker()
    {
        Speaker s = new Speaker();
        IBeepSpeakers wiredSpeakerBeeper =
            new IBeepSpeakers(s, s => ((Speaker)s).IsPlugged = true);
        wiredSpeakerBeeper.BeepTheSpeaker();
    }

    public static BeepWirelessSpeaker()
    {
        WirelessSpeaker w = new WirelessSpeaker();
        IBeepSpeakers wirelessSpeakerBeeper =
            new IBeepSpeakers(w, s => ((WiredSpeaker)s).IsTransmitterOn = true);
        wirelessSpeakerBeeper.BeepTheSpeaker();
    }
}

As you can see, we actually didn't need Code Contracts at all to tell us whether or not the speaker should beep. No, rather we let the state of the object itself determine whether it can beep.

Answer 3

If you really wanted to have a variance in behavior like this, you would probably want to expose a virtual 'CanBeep' property in the base class, then implement it for WirelessSpeaker to return TransmitterIsOn. This way you can still put your contract in Speaker and consumers of Speaker have a way to know whether they can meet the contractual requirements.

That said, public properties that could potentially be tied to mutable state are not a great choice for contractual requirements. What happens if the transmitter goes off between checking the property and calling the method? It's important to think carefully about the meaning of contracts, I think. A good question to ask is: is this a condition that I can prove statically, at compile time, or could it depend on runtime conditions? Incidentally, this question is most easily answered by running the static contract analysis tool.