Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

Looking at the java.util.Collections.unmodifiableMap implementation (OpenJDK 11):

    /**
     * Returns an <a href="Collection.html#unmodview">unmodifiable view</a> of the
     * specified map. Query operations on the returned map "read through"
     * to the specified map, and attempts to modify the returned
     * map, whether direct or via its collection views, result in an
     * {@code UnsupportedOperationException}.<p>
     *
     * The returned map will be serializable if the specified map
     * is serializable.
     *
     * @param <K> the class of the map keys
     * @param <V> the class of the map values
     * @param  m the map for which an unmodifiable view is to be returned.
     * @return an unmodifiable view of the specified map.
     */
    public static <K,V> Map<K,V> unmodifiableMap(Map<? extends K, ? extends V> m) {
        return new UnmodifiableMap<>(m);
    }

My question is why does the implementation not do a check that the map passed might already be an UnmodifiableMap, something like this :

    public static <K,V> Map<K,V> unmodifiableMap(Map<? extends K, ? extends V> m) {
        if(m instanceof UnmodifiableMap){
            return m;
        }
        return new UnmodifiableMap<>(m);
    }

Rather this question can be extended to all other un-modifiable collections, a simple check helps to avoid unwanted stackoverflow errors as well as unnecessary wrapping.

I wanted to know if there was a reason why this is not being done?

Also it is sort of impossible (without using Reflection/Classloader magic) to do the check by the user as UnmodifiableMap is private.

like image 591
Adwait Kumar Avatar asked Dec 29 '19 07:12

Adwait Kumar


1 Answers

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]));
    }
}
like image 163
Eugene Avatar answered Sep 19 '22 07:09

Eugene