7

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.

  • 2
    See here and here. – Euphoric Apr 22 '16 at 7:59
  • Neither NpcParty nor PlayerParty seem to extend Party. – Tulains Córdova Apr 22 '16 at 13:54
  • @TulainsCordova Apologies, I've updated the code now. – Zymus Apr 22 '16 at 14:19
  • Your test example just tests the different behaviour of the constructors - but as I already wrote in my answer, that is not what the LSP is about - LSP says an NpcParty object behaves the same as each Party object - it does not say the constructors behave the same. Read the links given by Euphoric for getting a better understanding of that. – Doc Brown Apr 22 '16 at 23:07
8

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.

| improve this answer | |
  • The only class that would be able to modify the input collection would be the PlayerParty. All NpcParty objects would be essentially immutable. Typical use-case would be to load the Foos from a repository, and would not be reassigned during runtime. If one wanted to change a value inside one of the Foos, they would have to do it in the repository. Only the Foos inside PlayerPartys can be modified during runtime, either in order, or reference. – Zymus Apr 22 '16 at 7:05
  • @DocBrown "having a derived class with a stronger constraints would violate the LSP" - I'd rather say with conflicting constraints, e.g. ones that make it impossible to satisfy Party requirements and NpcParty requirements at the same time. If you satisfy a stronger constraint, you automatically satisfy the weaker one by definition and pass the LSP test successfully. – guillaume31 Apr 22 '16 at 7:13
  • 1
    @DavidPacker your example isn't substitution to me. new NpcParty(collection) wouldn't work in the first place, regardless if you assign it to a Party or anything else. – guillaume31 Apr 22 '16 at 7:22
  • 3
    @DavidPacker it seems to me that the premise of LSP is to have both a valid instance of type T and a valid instance of subtype S, otherwise it makes no sense trying to reason about what happens when you replace one with the other. – guillaume31 Apr 22 '16 at 7:35
  • 1
    @Evan The provable property is not that the size can be zero, it's that it is greater than or equal to zero. The property is provable for any instances of Party or NpcParty. – guillaume31 Apr 22 '16 at 9:23
1

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).

| improve this answer | |

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.