Why does Collections.unmodifiableMap not check if the map passed is already an UnmodifiableMap?

IMHO there's no good reason for omitting the check. One comment says

Many other unmodifiable map types can exist.

but this is no good reason either. With someone using a different implementation, the check would be ineffective, but still no problem.

The only reason for not doing the check can be performance. However, an instanceof (or class equality) check is pretty cheap and the eliminated indirection can easily offset

IMHO the current state is a bug; the check should be done, especially because of UnmodifiableMap being private, so that the test can't be done in user code.

OTOH the cases when this matters are rare and Java is very conservative, so I don't think it ever gets fixed. You may want to check the bug database to see whether this issue has been reported.


I would assume the reason your proposed check is not being done is that creating an instance of an UnmodifiableMap is really just creating a thin wrapper around the underlying map instance, rather than a deep copy. To create a deep unmodifiable copy, you would have to do something like this:

Map<String, String> unmodMap = Collections.unmodifiableMap(new HashMap<>(yourMap));

If this were the implementation, then checking if the map reference already points to an UnmodifiableMap could perhaps avoid the need to make a deep copy.

There may not be very much performance gain in avoiding wrapping an existing unmodifiable map a second (or third) time, so to keep the implementation simple, the creators just chose to not bother checking.


I always saw that as somehow weird too, as a matter of fact when you do almost the same logical thing with Java 9 or newer, via:

    Map<String, Integer> left = Map.of("one", 1);
    Map<String, Integer> right = Map.copyOf(left);
    System.out.println(left == right); // true

you could see that the implementation does a check to see if this Map is known to be immutable already:

static <K, V> Map<K, V> copyOf(Map<? extends K, ? extends V> map) {
    if (map instanceof ImmutableCollections.AbstractImmutableMap) {
        return (Map<K,V>)map;
    } else {
        return (Map<K,V>)Map.ofEntries(map.entrySet().toArray(new Entry[0]));
    }
}