Nicolas
Nicolas

Reputation: 1484

SequencedCollection interface in Java not following SOLID principles

In Java SE 21 there is an interface called SequencedCollection.

There are few built-in classes that implement that interface, including ArrayList and TreeSet.

The interface defines some methods, for example addLast, amongst many others, although this one will suffice.

If I use an ArrayList type, the code will compile and will run correctly:

var myArray = new ArrayList<String>(List.of("a", "b"));
myArray.addLast("c");

If I use a TreeSet, this code will compile, but will throw a runtime exception:

var myTree = new TreeSet<String>(List.of("a", "b"));
myTree.addLast("c");

This is because the method addLast is not available to the TreeSet even though it implements the same interface.

This is inconvenient, because if I have a method that receives a SequencedCollection class type as parameter, I need to second guess within the method body (i.e. instanceof) whether the actual object passed as parameter is the one I can work with.

That is another way of saying that some children of SequencedCollection can't be used in place of its parent class without unexpected behaviour.

This in turn is another way of saying that SequencedCollection interface violates the Liskov Substitution principle (the L in SOLID).

Did I understand the Liskov Substitution principle by applying it in this particular example?

Upvotes: 3

Views: 105

Answers (1)

rzwitserloot
rzwitserloot

Reputation: 103608

It is invalid; liskov is not being broken.

That's because the implementation of addLast in TreeSet adheres perfectly well to the spec. The spec says, and I quote from the javadoc of SequencedCollection:

default void addLast(E e)

Adds an element as the last element of this collection (optional operation).

Throws:

UnsupportedOperationException - if this collection implementation does not support this operation

Hence, the implementation of addLast in TreeSet successfully fulfills this contract. It does not support that operation and signals this by throwing the stated exception.

Kicking the can down the road

We now get to a different problem I'm sure you're going to have with this getup: The spec of addLast is utterly useless, because it allows implementors to successfully implement it with a useless implementation, and in addition, it is not possible to, via the type system, figure out if a given instance of SequencedCollection has a 'useful' or a 'useless' implementation.

However. Your extensive quoting of officious sounding words such as 'SOLID' and 'Liskov Substitution Principle' indicates you like a world defined by objective absolutes. And in that world, there's nothing wrong with it; you can't say that 'just throwing an exception instead of adding an element at the end is not useful'; that would be a subjective conclusion.

Hence, you have to bring subjectivity into it.

So let's do that.

This is, subjectively, fine.

Java's collection APIs have worked this way since v1.0 of those APIs; introduced with java 1.2 if memory serves, well over 20 years ago: All the collection-related top level types, for example:

  • List, which has, for example, clear().
  • Iterator, which has, for example, remove().
  • Collection, which has, for example, removeAll.

They've always had these methods. Immutable collections, as a concept, cannot support these methods. (No, you cannot implement them by creating clones; these methods explicitly specify that they return void or boolean; there is no room to 'implement' a mutable operation by making a new collection with the operation applied and returning it).

There are 5 options available; they all break various style rules; just, different ones, and in different ways:

  • Fork the API; create new java.util.ImmutableCollection types which is bizarre, in that the 'mutable' side gets to 'claim' the simple word (just Collection), and this clashes with liskov. Is java.util.Collection (the mutable one) as subtype of ImmutableCollection? Liskov says yes. Common sense, spec definitions, and pragmatics say no. On its face, it would be a lie: ArrayList implements Collection.. and therefore ImmutableCollection. yet, ArrayList is not immutable. What name should one give to this notion of 'a Collection where write ops are not guaranteed, therefore, the interface does not provide them?' without implying that instances of it are, in fact, immutable? Whilst subjective, 'really bad misleading names' is a vastly more egregious problem with an API that a violation of Liskov. (We do not have to agree on this, but the java community agrees on this, as do the primary decision makers; hence if you disagree and want to get ideological about it, java is not the language for you).

  • Just redefine the API. Delete remove() from Collection. This is backwards incompatible. We're back: If you find Liskov more important than backwards compatibility, do not use java. You're surrounded by, and the language is shepherded by, people who strongly chose a side and pick backwards compatibility as being much more important.

  • Add a flag interface that tells you the type is, in fact, immutable (or alternatively, one that 'flags' that it is mutable). This doesn't actually solve the Liskov dilemma. In theory you could then write:

void addStuff(List<String> list) {
  if (!(list instanceof MutableList)) throw new IllegalArgumentException("mutable list required");
  // code as normal here
}

But this doesn't accomplish anything useful: Just write your code as normal, the .add method of an immutable list already throws a meaningful exception.

One could instead state that classes like ArrayList should implements MutableList<T> where that is defined as public interface MutableList<T> extends List<T> {}, but, this is backwards incompatible too: Existing list implementations (it's not a sealed interface; any library can provide implementations and many do) don't just up and change what they implement because java's engineers have a bullhorn and a dictatorial decree to force every maintainer into it.

And now you have a bizarre situation where there is a MutableList type, but the mutable ops aren't defined there, they are defined on just List. List would still be breaking Liskov. The 'win' is effectively zilch.

You can try to tie yourself into knots and find a way to introduce into the type system a way to use a specific 'mutable collection' concept that actually guarantees it, but you can't introduce that without breaking stuff. Consider things like Arrays.asList, Collections.unmodifiableList, List.of, and so forth.

  • Just accept it; document those methods that are optional, explain why they are optional (immutable types cannot and should not attempt to provide implementations), explain what one should do instead (throw UnsupportedOperationException), and otherwise, move on. This, obviously, breaks LSP except for the 'trick' that technically it doesn't as the spec now explicitly says throwing UOE is one way to implement it.

  • Abandon java.util entirely and just start the collection stuff over from scratch. No effort to attempt to reconcile the old way and the new way other than perhaps one utility class that lets you convert old stuff to new stuff, but that's where it ends. But this is tantamount to introducing a complete split in the java ecosystem; an 'old' and a 'new'. And this is therefore decreeing in one fell swoop that everything is now backwards incompatible. The burden on the ecosystem is herculean, and as even the relatively 'low impact' break from Python2 to Python3 shows, the pain of that transition will last for decades.


The OpenJDK maintainers chose the 4th option.

Upvotes: 2

Related Questions