2

I was recently reading Java Concurrency in Practice and was exposed to the Collections.unmodifiableMap(...) method for the first time. The method creates a read-only wrapper around an existing Map and any attempts to modify the returned Map will (according to the Javadocs) result in an UnsupportedOperationException being thrown. Similar methods exist for other collection classes.

This made me quite concerned since an unmodifiableMap() still returns a Map, but does not support all relevant methods. The fact that it also throws exceptions on write operations means it cannot substitute a "proper" Map in most applications.

I am a student and am not yet confident in my ability to recognize design flaws, but aren't these violations of the Interface segregation and Liskov substitution principles, respectively?

Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
nylanderDev
  • 531
  • 4
  • 14
  • It strikes me that it might not violate the I-principle, as that pertains to the creator of the interfaces, not the clients. – nylanderDev Jun 17 '20 at 01:41
  • 1
    The `Map` interface documents that implementations may choose not to support all of its methods. Honestly, it's a design compromise that works just fine in practice. – Louis Wasserman Jun 17 '20 at 02:48
  • @LouisWasserman Thanks, I had missed that. If you make it an answer I'll close the question. – nylanderDev Jun 17 '20 at 02:49

2 Answers2

5

The Map interface documents that implementations may choose not to support all of its methods, which makes Collections.unmodifiableMap return an implementation that satisfies the interface contract.

While it's unusual for an interface to make implementing its methods "optional" in this way, it's a design compromise that works just fine in practice. Most collections should be written only once and then read from again and again, so code that modifies a map is usually the code that created it and knows that it is mutable.

Louis Wasserman
  • 191,574
  • 25
  • 345
  • 413
  • 1
    I would have preferred a separation of `Map` and `MutableMap` where the latter extends the former (same with the other collections). The mutable versions should also have a method which returns a read-only view (e.g. `Map asReadOnlyMap()`), possibly returning a cached instance (up to the implementation), to prevent any downcasting when exposing said collection. – Slaw Jun 17 '20 at 04:14
  • @Slaw, you may be interested in http://blog.codefx.org/java/immutable-collections-in-java/. – jaco0646 Jun 17 '20 at 13:28
  • @jaco0646 Interesting blog :) It basically ends up at the same solution I would have preferred, just with an extra _immutable_ version of the interface as well (I was only going with read-only/read-write in the same vein as unmmodifiable collections now, just without the confusion of mutator methods being present in read-only _references_). Though I agree, unfortunately, with the poster that this will never be implemented in Java due to it breaking backwards compatibility. – Slaw Jun 17 '20 at 16:58
  • 1
    The first practical problem that comes up with that is the type of `MutableMap.keySet`, which can support removals but not addition, or `Arrays.asList`, which supports `set` but not `add` or `remove`. Capturing that level of detail in explicit interfaces becomes impractical fast. – Louis Wasserman Jun 17 '20 at 22:04
  • @Louis Yes, there are still some issues and some methods will still have to be documented to throw `UnsupportedOperationException`, but I would have preferred such "half measures" instead of the complete lack of read-only/immutable collection types. – Slaw Jun 18 '20 at 00:25
2

Regarding the ISP, Bob Martin would likely consider Collections.unmodifiableMap() to produce a violation in the same vein as his stack class example. It exposes clients to an interface they do not need, and in fact cannot use.

As previously mentioned, the LSP is satisfied by the interface documentation.

jaco0646
  • 15,303
  • 7
  • 59
  • 83