5
votes

Pourquoi Collections.unmodifiableMap ne vérifie-t-il pas si la carte passée est déjà une UnmodifiableMap?

En regardant l'implémentation de java.util.Collections.unmodifiableMap (OpenJDK 11):

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

Ma question est de savoir pourquoi l'implémentation ne vérifie pas que la carte passée pourrait déjà être une carte UnmodifiableMap , quelque chose comme ceci:

    /**
     * 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);
    }

Au contraire, cette question peut être étendue à toutes les autres collections non modifiables, une simple vérification permet d'éviter les erreurs de dépassement de pile indésirables ainsi que l'encapsulation inutile.

Je voulais savoir s'il y avait une raison pour laquelle cela ne se fait pas?

De plus, il est en quelque sorte impossible (sans utiliser la magie de Reflection / Classloader) de faire la vérification par l'utilisateur car UnmodifiableMap est privé.


3 commentaires

UnmodifiableMap se trouve être ce que cette implémentation et l' implémentation actuelle de la méthode retournent. De nombreux autres types de cartes non modifiables peuvent exister. Et, cette implémentation de la méthode unmodifiableMap peut changer à l'avenir pour renvoyer une classe différente ... Je pense que faire m instanceof UnmodifiableMap laisserait plus de questions qu'elle ne répondrait (oui, car il n'y a pas d'API / contrat pour l'attribut `` non modifiable '').


Au cas où ils voudraient échanger l'implémentation avec quelque chose d'autre un jour, ils peuvent aller de l'avant et changer le chèque pour cette implémentation ce jour-là. Y a-t-il un problème avec cela?


D'accord. Sauf que des tests sur des cartes sérialisées sur l'ancienne version conduiraient à des résultats imprévisibles. Ce n'est pas un gros problème, mais je suppose que le gain de performance ne serait pas beaucoup de toute façon


3 Réponses :


3
votes

Je suppose que la raison pour laquelle la vérification que vous proposez n'est pas effectuée est que la création d'une instance d'un UnmodifiableMap n'est en réalité que la création d'un mince wrapper autour de l'instance de carte sous-jacente, plutôt qu'une copie complète. Pour créer une copie non modifiable profonde, vous devez faire quelque chose comme ceci:

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

S'il s'agissait de l'implémentation, vérifier si la référence de la carte pointe déjà vers un UnmodifiableMap pourrait peut-être éviter d'avoir à faire une copie complète.

Il se peut qu'il n'y ait pas beaucoup de gain de performances en évitant d'encapsuler une carte existante non modifiable une deuxième (ou une troisième) fois, donc pour que l'implémentation reste simple, les créateurs ont simplement choisi de ne pas prendre la peine de vérifier.


3 commentaires

Je comprends que cela ne crée pas de copie complète, mais je ne vois pas pourquoi l'ajout de la vérification de la carte passée complique les choses. Des personnes ont déjà été brûlées en se heurtant à un débordement de pile à cause de cela.


@AdwaitKumar Ce débordement de pile (l'erreur, pas le site Web) sonne comme le résultat d'une mauvaise programmation récursive, où les auteurs ne prêtaient pas attention au nombre de fois qu'ils créent une instance de carte non modifiée. Ce ne devrait pas être le travail de cette classe de gérer de tels cas.


Pas seulement de la programmation récursive, j'ai vu des modèles (bien que ce ne soit peut-être pas un bon modèle) dans des bases de code où les fonctions qui prennent une Map , l'enveloppent via Collections#unmodifiableMap pour s'assurer qu'elles ne modifient pas la carte par erreur. Cela se produit indépendamment dans plusieurs classes et conduit parfois à un encapsulation trop importante en raison de la profondeur des appels de fonction.



3
votes

IMHO, il n'y a aucune bonne raison d'omettre le chèque. Un commentaire dit

De nombreux autres types de cartes non modifiables peuvent exister.

mais ce n'est pas non plus une bonne raison. Avec quelqu'un utilisant une implémentation différente, le contrôle serait inefficace, mais toujours pas de problème.

La seule raison de ne pas faire le contrôle peut être la performance. Cependant, une vérification d' instanceof (ou d'égalité de classe) est assez bon marché et l'indirection éliminée peut facilement compenser

IMHO l'état actuel est un bogue; la vérification doit être effectuée, en particulier parce que UnmodifiableMap est private , de sorte que le test ne peut pas être fait dans le code utilisateur.

OTOH les cas où cela compte sont rares et Java est très conservateur, donc je ne pense pas que cela soit jamais corrigé. Vous voudrez peut-être vérifier la base de données de bogues pour voir si ce problème a été signalé.


2 commentaires

Oh, c'est une autre chose que j'ai complètement oublié qu'il est en quelque sorte impossible pour l'utilisateur de faire la vérification.


Je suis d'accord avec vous, et java-9 fait la vérification en interne, par exemple



1
votes

J'ai toujours vu cela aussi bizarre, en fait, quand vous faites presque la même chose logique avec Java 9 ou plus récent, via:

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]));
    }
}

vous pouvez voir que l'implémentation effectue une vérification pour voir si cette Map est déjà connue pour être immuable:

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


0 commentaires