4

I answered a question regaurding an ImmutableMap. I suggested using the Proxy pattern.

The problem with this is that Map contains a put method, which would throw an UnsupportedOperationException. Replacing other instances of Map with ImmutableMap would break the Liskov Subsitution principle. Not only that, the need to declare put and putAll [violates the interface segregation principle]

Technically, there is no way to replace a Map instance with ImmutableMap, since Map is simply an interface. So my question is:

Would creating an ImmutableMap using the Map interface be considered breaking the LSP, since Map contains a put and putAll method? Would not implementing Map be considered an "Alternative Classes with Different Interfaces" code smell? How would one create an ImmutableMap that abides by the LSP yet doesn't contain code smells?

Bhargav Rao
  • 50,140
  • 28
  • 121
  • 140
Vince
  • 14,470
  • 7
  • 39
  • 84
  • 2
    [`Map.put`](http://docs.oracle.com/javase/7/docs/api/java/util/Map.html?is-external=true#put%28K,%20V%29) and [`Map.putAll`](http://docs.oracle.com/javase/7/docs/api/java/util/Map.html#putAll%28java.util.Map%29) (as well as `remove` and `clear`) are defined as "optional operations". Also, the possibility of throwing an `UnsupportedOperationException` from implementing classes is specified in the interface documentation. – Mick Mnemonic Apr 25 '15 at 23:54
  • @MickMnemonic But that violates the [interface segregation principle](http://en.m.wikipedia.org/wiki/Interface_segregation_principle), which states "*no client should be forced to depend on methods it does not use*". I'll edit the question to include that it should follow other SOLID principles as well – Vince Apr 26 '15 at 00:01
  • 3
    My point was that the contracts for those mutating methods are at least well documented in the `Map` interface. But you have a very valid point; the `Map` interface is a _fat_ one. Perhaps it should have been split into `Map` and `ModifiableMap extends Map`, originally. – Mick Mnemonic Apr 26 '15 at 00:12
  • 1
    Sometimes you have to do what you have to do. Since Map is used all over the place you just throw in the unused methods. Nothing you can do there without inventing your own type hierachy (which you could do but nobody would provide it). – eckes Apr 26 '15 at 00:33
  • See http://stackoverflow.com/questions/22050848/do-collections-unmodifiablexxx-methods-violate-lsp for a debate on whether collection mutation methods violate LSP. – jaco0646 Aug 03 '16 at 16:07
  • @jaco0646 Personally, it's not really debatable. The contract specified in the javadoc is a cheap way of working around another principle they violated: interface segregation (`List` should not have `add` in the first place, since not all `List` objects can be added to). Kotlin is a fine example of how it should have been handled: segregating the `add` method to an `MutableList`, and keeping `List` immutable. Kotlin actually maps Java collections to their own collections (when interoperating between the two) at compile time due to this. – Vince Aug 03 '16 at 19:14

1 Answers1

1

In my view, an ImmutableMap should implement Map. It would be a bad idea not to implement Map as there are many methods that accept a Map as an argument and only use it in a read-only sense. I don't believe this does violate the Liskov Subsitution principle because the contract for Map makes it clear that put is an optional operation.

It is not ideal that classes implementing Map have to implement put, but the alternative would have been to have a complex hierarchy of interfaces each only including a subset of the possible optional methods. If there are n optional methods, there would have to be 2^n interfaces to cover all the combinations. I don't know the value of n, but there are some non-obvious optional operations, such as whether or not the Iterator returned by map.entrySet().iterator() supports the setValue operation. If you combined this hierarchy with the hierarchy of interfaces and abstract classes that actually exists already (including AbstractMap, SortedMap, NavigableMap, ConcurrentMap, ConcurrentNavigableMap...) you would have a total mess.

So there is no perfect answer to this, but in my view the best solution is to make ImmutableMap implement Map and ensure that every method you write with a Map as an argument clearly documents any properties the Map must have and the exceptions thrown if the wrong type of Map is passed.

Paul Boddington
  • 37,127
  • 10
  • 65
  • 116
  • 1
    A hierarchy of interfaces would abide the interface segregation principle; I'd much prefer many interfaces to a single bulky one. I can think of a few cases where a `Map` is mutated, but you're right; most cases involve only accessing (at least in situations common to me). I'm not gonna lie, I already figured such an answer; I was really hoping for some kind of "hidden gem" that could help me solve this situation. I will wait a little more for more answers before accepting; I hope you understand. If none appear, you'll get my vote, due to your calming statement :) – Vince Apr 26 '15 at 00:49