How to resolve LSP violation based on minimum precondition

Question

I have a class Party that has a constructor that takes a Collection<Foo>. I plan to have two subclasses NpcParty and PlayerParty. All Party instances have an upper limit on the size of the input collection (6). However, an NpcParty has a lower bound of 1, while a PlayerParty has a lower bound of 0 (or rather, there is an implied minimum bound because a List cannot have a negative size). This is a violation of LSP because the NpcParty strengthens the preconditions in terms of the size of the input collection.

An NpcParty is intended to be immutable; meaning that the only Foos it will ever have, are those that are specified in the constructor. A PlayerParty's Foos can be changed at runtime, either in order, or reference/value.

public class Party {

    // collection must not be null
    // collection size must not exceed PARTY_LIMIT
    public Party(List<Foo> foos) {
        Objects.requireNonNull(foos);

        if (foos.size() > PARTY_LIMIT) {
            throw new IllegalArgumentException("foos exceeds party limit of " + PARTY_LIMIT);
        }
    }
}

public class NpcParty extends Party {

    // additional precondition that there must be at least 1 Foo in the collection
    public NpcParty(List<Foo> foos) {
        super(foos);

        if (foos.size() < 1) {
            throw new IllegalArgumentException("foos must contain at least 1 Foo");
        }
    }
}

public class PlayerParty extends Party {

    // no additional preconditions
    public PlayerParty(List<Foo> foos) {
        super(foos);
    }
}

In what ways can I resolve this violation, such that an NpcParty is allowed to have a minimum bound?

EDIT: I'm going to give an example of how I might test this.

Suppose I had a class AbstractPartyUnitTests that tested the minimum contract of all Party implementations.

public class AbstractPartyUnitTests {

    @Test(expected = NullPointerException.class)
    public void testNullConstructor() {
        createParty(null);
    }

    @Test
    public void testConstructorWithEmptyList() {
        party = createParty(new ArrayList<Foo>());

        assertTrue(party != null);
    }

    @Test(expected = IllegalArgumentException.class)
    public void testConstructorThatExceedsMaximumSize() {
        party = createParty(Stream.generate(Foo::new)
                                  .limit(PARTY_LIMIT + 1)
                                  .collect(Collectors.toList()));
    }

    protected abstract Party createParty(List<Foo> foos);

    private Party party;
}

With subclasses for PlayerParty and NpcParty

public class PlayerPartyUnitTests extends AbstractPartyUnitTests {

    @Override
    protected Party createParty(List<Foo> foos) {
        return new PlayerParty(foos);
    }
}

and

public class NpcPartyUnitTests extends AbstractPartyUnitTests {

    @Test
    public void testConstructorThatMeetsMinimumSize() {
        party = createParty(Stream.generate(Foo::new)
                                  .limit(1)
                                  .collect(Collectors.toList());

        assertTrue(party != null);
    }

    @Test(expected = IllegalArgumentException.class)
    public void testConstructorThatDoesNotMeetMinimumSize() {
        party = createParty(new ArrayList<Foo>());
    }

    @Override
    protected Party createParty(List<Foo> foos) {
        return new NpcParty(foos);
    }
}

All of the test cases would pass, with the exception of testConstructorWithEmptyList while running as an NpcPartyUnitTests. The constructor would fail, and as such, the test would fail.

Now, I could remove that test from the AbstractPartyUnitTests class, as it does not really apply to all types of Party; but then anywhere I have a Party, I may not be able to have a 1:1 replacement of NpcParty.

Doc Brown · Answer 1 · 2016-04-22T15:04:20.300

In your example above there is actually no violation of the LSP. Each NpcParty object is still a valid Party object and can be used for it in exchange. The LSP is not about exchanging class A by class B, it is about exchanging objects of type A by objects of type B. Thus, constructors, and constraints which are only checked in there, are not subject to the LSP.

For example, lets assume Player has an additional property "Size". You try to write a test case where you inject the player from outside, and want to test if that size is always in the allowed bounds:

 void TestSize(Player p)
 {
       AssertTrue(p.Size>=0);
       AssertTrue(p.Size<=PARTY_LIMIT);

       // ??? try to write a test for "Size" which does not fail for
       // ordinary Player objects, or PlayerParty objects, but fails for NpcPLayer
       // -> not possible without explicitly checking the type
 }

As you see, whatever you pass into that test method, the test does not fail. There is nothing in here where you cannot replace ordinary Player objects by NpcPlayer Objects.

If, however, your Party class would store its input somewhere and provides methods to change the number of elements afterwards (checking a constraint that the number of elements has to greater zero), then having a derived class with a stronger constraints would violate the LSP, since then you could not use an NpcParty object as an exchange for a Party.

To solve this, implement Party more general - inject the minimum and maximum bounds for the collection by the constructor and store them somewhere. That way, each user of a Party object must not assume that these bounds are zero and PARTY_LIMIT - that makes NpcParty a valid exchange for Party.

Evan · Answer 2 · 2016-04-22T08:23:28.200

Your code as stated is not necessarily a violation of the Liskov Substitution Principle: you can introduce an additional constraint in the constructor as long as instantiated NpcParty objects do not assume that their foos collections always have at least one member.

It doesn't seem like that will be the case, and you'll probably want to rely on that property of NpcParty objects in your application's business logic. If I'm right, you are correct that, in principle, this is a violation of the Liskov Substitution Principle.

You can fix this violation by moving any behavior that relies on party size constraints to the subclasses: PlayerParty should define a constraint on its party size just like NpcParty does now, and Party should make no claims about party size at all (or at the very least, only make claims that are true for all of its subtypes).

How to resolve LSP violation based on minimum precondition

2 Answers2