C# LSP Constructor Parameters and Guard Clauses

Question

I've been reading about the Liskov Substitution Principle (LSP) and I'm a little confused on how you adhere to it correctly. Especially when interfaces and subclasses are being used.

For example, if I have a base class:

public abstract class AccountBase
{
    private string primaryAccountHolder;

    public string PrimaryAccountHolder 
    { 
        get { return this.primaryAccountHolder; } 
        set
        {
            if (value == null) throw ArgumentNullException("value");
            this.primaryAccountHolder = value;
        }
    }

    public string SecondaryAccountHolder { get; set; }

    protected AccountBase(string primary)
    {
        if (primary == null) throw new ArgumentNullException("primary");
        this.primaryAccountHolder = primary;
    }
}

Now let's say I have two accounts that inherit from the base class. One that REQUIRES the SecondaryAccountHolder. Adding a null guard to the sub-class is a violation of LSP, correct? So how would I design my classes in such a way that they don't violate LSP but one of my sub-classes requires a secondary account holder and one does not?

Compound the question with the fact that there could be tons of different types of accounts and they'll need to be generated through a factory or factory that returns a builder or something.

And I have the same question with interfaces. If I have an interface:

public interface IPrintsSomething
{
    void PrintSomething(string text);
}

Wouldn't it be a violation of LSP to add a null guard clause for text on any class that implements IPrintsSomething? How do you protect your invariants? That is the correct word right? :p

Answer 1

So how would I design my classes in such a way that they don't violate LSP but one of my sub-classes requires a secondary account holder and one does not?

The way out of this problem is by surfacing this variability to the contract of the base class. It may look like this (unnecessary implementation details left out):

public abstract class AccountBase
{
    public string PrimaryAccountHolder 
    { 
        get { … } 
        set { … }
    }

    public string SecondaryAccountHolder
    { 
        get { … } 
        set 
        { 
            …  
            if (RequiresSecondaryAccountHolder && value == null) throw …;
            …  
        } 
    }

    public abstract bool RequiresSecondaryAccountHolder { get; }
}

Then you are not violating the LSP, because the user of AccountBase can determine whether they have to or have not to provide the value of SecondaryAcccountHolder.

And I have the same question with interfaces. … Wouldn't it be a violation of LSP to add a null guard clause for text on any class that implements IPrintsSomething?

Make the validation an obvious part of the interface's contract. How? Document, that the implementor must chek the value of text for null.

Answer 2

You should research tell-don't-ask, and command/query separation, you could start here: https://pragprog.com/articles/tell-dont-ask

You should endeavor to tell objects what you want them to do; do not ask them questions about their state, make a decision, and then tell them what to do.

There's always something you want to do with the properties, well don't ask the object for them tell it to do something with them.

Instead of asking it and making decisions like this:

string holders = account.PrimaryAccountHolder;
if (accountHolder.SecondaryAccountHolder != null)
{
    holders += " " + accountHolder.SecondaryAccountHolder;
}

Tell it:

string holders = account.ListAllHoldersAsAString();

Ideally, you'd actually tell it what you actually want to do with that string:

account.MailMergeAllAccountHoldersNames(letterDocument);

Now the logic for dealing with two account holders is in the subclass. Could be one, two or n account holders, the calling code doesn't care or need to know.

As for LSP, well if there's a formally (or informally) documented contract that says the clients must check for null on the second holder from the start then that's fine. It's not nice, but any null-pointer-exceptions will be the client's fault for not using the class correctly. (Note it's not true that adding a boolean property improves upon this, it's just maybe a little more readable, i.e. does anyone check IList.IsReadOnly before writing to it?!).

However, if you started with the double holder account and then added that condition that the second account holder can be null later for the single account, then you changed the contract, and an instance of the single could break existing code. If you're in full control of all places that use accounts, then you're allowed to do that, if that's a public api you're changing, that's a different matter.

But tell-don't-ask avoids the whole problem in this case.