Ist es besser, eine ImmutableMap oder eine Map zurückzugeben?

90

Angenommen, ich schreibe eine Methode, die eine Map zurückgeben soll . Zum Beispiel:

public Map<String, Integer> foo() {
  return new HashMap<String, Integer>();
}

Nachdem ich eine Weile darüber nachgedacht habe, habe ich entschieden, dass es keinen Grund gibt, diese Karte nach ihrer Erstellung zu ändern. Daher möchte ich eine ImmutableMap zurückgeben .

public Map<String, Integer> foo() {
  return ImmutableMap.of();
}

Soll ich den Rückgabetyp als generische Map belassen oder angeben, dass ich eine ImmutableMap zurückgeben möchte?

Genau aus diesem Grund wurden Schnittstellen für erstellt. um die Implementierungsdetails auszublenden.
Auf der anderen Seite könnten andere Entwickler die Tatsache übersehen, dass dieses Objekt unveränderlich ist, wenn ich es so belasse. Somit werde ich kein Hauptziel unveränderlicher Objekte erreichen; um den Code klarer zu machen, indem die Anzahl der Objekte minimiert wird, die sich ändern können. Selbst das Schlimmste ist, dass nach einer Weile jemand versucht, dieses Objekt zu ändern. Dies führt zu einem Laufzeitfehler (der Compiler warnt nicht davor).

AMT
quelle
2
Ich vermute, irgendwann wird jemand diese Frage als zu offen für Argumente markieren. Können Sie spezifischere Fragen anstelle von "WDYT" stellen? und "ist es in Ordnung ...?" Beispiel: "Gibt es Probleme oder Überlegungen bei der Rückgabe einer regulären Karte?" und "Welche Alternativen gibt es?"
Asche
Was ist, wenn Sie später entscheiden, dass tatsächlich eine veränderbare Karte zurückgegeben werden soll?
user253751
3
@immibis lol oder eine Liste für diese Angelegenheit?
Djechlin
3
Möchten Sie wirklich eine Guava-Abhängigkeit in Ihre API einbinden? Sie können dies nicht ändern, ohne die Abwärtskompatibilität zu beeinträchtigen.
user2357112 unterstützt Monica
1
Ich denke, dass die Frage zu lückenhaft ist und viele wichtige Details fehlen. Zum Beispiel, ob Sie Guava ohnehin schon als (sichtbare) Abhängigkeit haben. Selbst wenn Sie es bereits haben, sind die Codefragmente Pesudocode und vermitteln nicht, was Sie tatsächlich erreichen möchten. Du würdest bestimmt nicht schreiben return ImmutableMap.of(somethingElse). Stattdessen würden Sie das ImmutableMap.of(somethingElse)als Feld speichern und nur dieses Feld zurückgeben. All dies beeinflusst das Design hier.
Marco13

Antworten:

70
  • Wenn Sie eine öffentlich zugängliche API schreiben und diese Unveränderlichkeit ein wichtiger Aspekt Ihres Entwurfs ist, würde ich dies definitiv explizit machen, indem der Name der Methode eindeutig angibt, dass die zurückgegebene Karte unveränderlich ist, oder indem der konkrete Typ von zurückgegeben wird die Karte. Es reicht meiner Meinung nach nicht aus, es im Javadoc zu erwähnen.

    Da Sie anscheinend die Guava-Implementierung verwenden, habe ich mir das Dokument angesehen und es ist eine abstrakte Klasse, sodass Sie ein wenig Flexibilität hinsichtlich des tatsächlichen, konkreten Typs erhalten.

  • Wenn Sie ein internes Tool / eine interne Bibliothek schreiben, ist es viel akzeptabler, nur eine Ebene zurückzugeben Map. Die Leute kennen die Interna des Codes, den sie aufrufen, oder haben zumindest einfachen Zugriff darauf.

Mein Fazit wäre, dass explizit gut ist, überlasse die Dinge nicht dem Zufall.

Dici
quelle
1
Eine zusätzliche Schnittstelle, eine zusätzliche abstrakte Klasse und eine Klasse, die diese abstrakte Klasse erweitert. Dies ist zu viel Aufwand für eine so einfache Sache, wie sie das OP verlangt, wenn die Methode den MapSchnittstellentyp oder den ImmutableMapImplementierungstyp zurückgeben soll.
fps
20
Wenn Sie sich auf Guava beziehen, empfiehlt ImmutableMapdas Guava-Team, ImmutableMapeine Schnittstelle mit semantischen Garantien in Betracht zu ziehen und diesen Typ direkt zurückzugeben.
Louis Wasserman
1
@ LouisWasserman mm, habe nicht gesehen, dass die Frage einen Link zur Guava-Implementierung enthält. Ich werde das Snippet dann entfernen, danke
Dici
3
Ich stimme Louis zu, wenn Sie beabsichtigen, eine Karte zurückzugeben, die ein unveränderliches Objekt ist, stellen Sie sicher, dass das Rückgabeobjekt vorzugsweise von diesem Typ ist. Wenn Sie aus irgendeinem Grund die Tatsache verschleiern möchten, dass es sich um einen unveränderlichen Typ handelt, definieren Sie Ihre eigene Schnittstelle. Es als Karte zu belassen ist irreführend.
WSimpson
1
@WSimpson Ich glaube, das ist es, was meine Antwort sagt. Louis sprach über einen Ausschnitt, den ich hinzugefügt hatte, aber ich entfernte ihn.
Dici
38

Sie sollten ImmutableMapals Rückgabetyp haben. Mapenthält Verfahren , die durch die Umsetzung des nicht unterstützt werden ImmutableMap( zum Beispiel put) , und sind gekennzeichnet @deprecatedin ImmutableMap.

Die Verwendung veralteter Methoden führt zu einer Compiler-Warnung und die meisten IDEs warnen, wenn Benutzer versuchen, die veralteten Methoden zu verwenden.

Diese erweiterte Warnung ist Laufzeitausnahmen vorzuziehen, da dies Ihr erster Hinweis darauf ist, dass etwas nicht stimmt.

Synesso
quelle
1
Dies ist meiner Meinung nach die vernünftigste Antwort. Die Kartenschnittstelle ist ein Vertrag, und ImmutableMap verstößt eindeutig gegen einen wichtigen Teil davon. Die Verwendung von Map als Rückgabetyp ist in diesem Fall nicht sinnvoll.
TonioElGringo
@TonioElGringo was ist Collections.immutableMapdann?
OrangeDog
@TonioElGringo Bitte beachten Sie, dass die Methoden, die die ImmutableMap nicht implementiert, in der Dokumentation der Schnittstelle docs.oracle.com/javase/7/docs/api/java/util/… explizit als optional markiert sind . Daher denke ich, dass es immer noch sinnvoll sein kann, eine Karte (mit richtigen Javadocs) zurückzugeben. Obwohl ich mich aus den oben genannten Gründen dafür entscheide, die ImmutableMap explizit zurückzugeben.
AMT
3
@TonioElGringo es verletzt nichts, diese Methoden sind optional. Wie in Listgibt es keine Garantie, List::adddie gültig ist. Versuchen Sie es Arrays.asList("a", "b").add("c");. Es gibt keinen Hinweis darauf, dass es zur Laufzeit fehlschlägt, dies ist jedoch der Fall. Zumindest gibt dir Guave einen Kopf hoch.
NJZK2
14

Auf der anderen Seite könnten andere Entwickler die Tatsache übersehen, dass dieses Objekt unveränderlich ist, wenn ich es so belasse.

Das sollten Sie in den Javadocs erwähnen. Entwickler lesen sie, wissen Sie.

Somit werde ich kein Hauptziel unveränderlicher Objekte erreichen; um den Code klarer zu machen, indem die Anzahl der Objekte minimiert wird, die sich ändern können. Selbst das Schlimmste ist, dass nach einer Weile jemand versucht, dieses Objekt zu ändern. Dies führt zu einem Laufzeitfehler (der Compiler warnt nicht davor).

Kein Entwickler veröffentlicht seinen Code ungetestet. Und wenn er es testet, wird eine Ausnahme ausgelöst, bei der er nicht nur den Grund, sondern auch die Datei und Zeile sieht, in der er versucht hat, auf eine unveränderliche Karte zu schreiben.

Beachten Sie jedoch, dass nur das Mapselbst unveränderlich ist, nicht die darin enthaltenen Objekte.

tkausl
quelle
10
"Kein Entwickler veröffentlicht seinen Code ungetestet." Möchten Sie darauf wetten? Es würde weit weniger (meine Vermutung) Fehler in öffentlicher Software geben, wenn dieser Satz wahr wäre. Ich wäre mir nicht einmal sicher, ob "die meisten Entwickler ..." wahr wären. Und ja, das ist enttäuschend.
Tom
1
Okay, Tom hat recht, ich bin mir nicht sicher, was Sie sagen wollen, aber was Sie tatsächlich geschrieben haben, ist eindeutig falsch. (Auch: Anstoß für geschlechtsspezifische Inklusivität)
Djechlin
@ Tom Ich denke, Sie haben den Sarkasmus in dieser Antwort verpasst
Captain Fogetti
10

Wenn ich es so lasse, könnten andere Entwickler die Tatsache übersehen, dass dieses Objekt unveränderlich ist

Das stimmt, aber andere Entwickler sollten ihren Code testen und sicherstellen, dass er abgedeckt ist.

Trotzdem haben Sie 2 weitere Möglichkeiten, um dies zu lösen:

  • Verwenden Sie Javadoc

    @return a immutable map
  • Wählen Sie einen beschreibenden Methodennamen

    public Map<String, Integer> getImmutableMap()
    public Map<String, Integer> getUnmodifiableEntries()

    Für einen konkreten Anwendungsfall können Sie die Methoden sogar besser benennen. Z.B

    public Map<String, Integer> getUnmodifiableCountByWords()

Was kannst du sonst noch tun?!

Sie können a zurückgeben

  • Kopieren

    private Map<String, Integer> myMap;
    
    public Map<String, Integer> foo() {
      return new HashMap<String, Integer>(myMap);
    }

    Dieser Ansatz sollte verwendet werden, wenn Sie erwarten, dass viele Clients die Karte ändern und solange die Karte nur wenige Einträge enthält.

  • CopyOnWriteMap

    Copy-on-Write-Sammlungen werden normalerweise verwendet, wenn Sie sich damit befassen müssen
    Parallelität . Das Konzept hilft Ihnen aber auch in Ihrer Situation, da eine CopyOnWriteMap eine Kopie der internen Datenstruktur bei einer mutativen Operation erstellt (z. B. Hinzufügen, Entfernen).

    In diesem Fall benötigen Sie einen Thin Wrapper um Ihre Map, der alle Methodenaufrufe mit Ausnahme der Mutationsoperationen an die zugrunde liegende Map delegiert. Wenn eine Mutationsoperation aufgerufen wird, wird eine Kopie der zugrunde liegenden Karte erstellt, und alle weiteren Aufrufe werden an diese Kopie delegiert.

    Dieser Ansatz sollte verwendet werden, wenn Sie erwarten, dass einige Clients die Karte ändern.

    Leider hat Java keine solche CopyOnWriteMap. Möglicherweise finden Sie jedoch einen Dritten oder implementieren ihn selbst.

Zuletzt sollten Sie bedenken, dass die Elemente in Ihrer Karte möglicherweise noch veränderbar sind.

René Link
quelle
8

Geben Sie auf jeden Fall eine ImmutableMap zurück. Die Begründung lautet:

  • Die Methodensignatur (einschließlich des Rückgabetyps) sollte sich selbst dokumentieren. Kommentare sind wie Kundenservice: Wenn sich Ihre Kunden auf sie verlassen müssen, ist Ihr primäres Produkt defekt.
  • Ob etwas eine Schnittstelle oder eine Klasse ist, ist nur beim Erweitern oder Implementieren relevant. Bei einer bestimmten Instanz (einem bestimmten Objekt) weiß oder kümmert sich der Clientcode in 99% der Fälle nicht darum, ob es sich bei einer Schnittstelle oder einer Klasse handelt. Ich nahm zunächst an, dass ImmutableMap eine Schnittstelle ist. Erst nachdem ich auf den Link geklickt hatte, wurde mir klar, dass es sich um eine Klasse handelt.
Benito Ciaro
quelle
5

Das hängt von der Klasse selbst ab. Guavas ImmutableMapsoll kein unveränderlicher Blick in eine veränderliche Klasse sein. Wenn Ihre Klasse unveränderlich ist und eine Struktur hat, die im Grunde genommen eine ist ImmutableMap, geben Sie den Rückgabetyp ein ImmutableMap. Wenn Ihre Klasse jedoch veränderlich ist, tun Sie dies nicht. Wenn Sie dies haben:

public ImmutableMap<String, Integer> foo() {
    return ImmutableMap.copyOf(internalMap);
}

Guave kopiert die Karte jedes Mal. Das ist langsam. Aber wenn internalMapes schon ein war ImmutableMap, dann ist es völlig in Ordnung.

Wenn Sie Ihre Klasse nicht auf die Rückkehr beschränken ImmutableMap, können Sie stattdessen Collections.unmodifiableMapwie folgt zurückkehren:

public Map<String, Integer> foo() {
    return Collections.unmodifiableMap(internalMap);
}

Beachten Sie, dass dies unveränderlich ist Ansicht in der Karte ist. Wenn sich dies internalMapändert, wird auch eine zwischengespeicherte Kopie von geändert Collections.unmodifiableMap(internalMap). Ich bevorzuge es jedoch immer noch für Getter.

Justin
quelle
1
Dies sind gute Punkte, aber der Vollständigkeit halber gefällt mir diese Antwort, die erklärt, dass sie unmodifiableMapnur vom Inhaber der Referenz nicht geändert werden kann - es handelt sich um eine Ansicht, und der Inhaber der Hintergrundkarte kann die Hintergrundkarte ändern, und dann ändert sich die Ansicht. Das ist also eine wichtige Überlegung.
Davidbak
@ Wie Davidback kommentiert, seien Sie sehr vorsichtig bei der Verwendung Collections.unmodifiableMap. Diese Methode gibt eine Ansicht auf die ursprüngliche Karte zurück, anstatt ein separates neues Kartenobjekt zu erstellen. Jeder andere Code, der auf die ursprüngliche Karte verweist, kann sie ändern, und dann lernt der Inhaber der angeblich nicht veränderbaren Karte auf die harte Tour, dass die Karte tatsächlich unter ihnen geändert werden kann. Ich bin selbst von dieser Tatsache gebissen worden. Ich habe gelernt, entweder eine Kopie der Karte zurückzugeben oder Guava zu verwenden ImmutableMap.
Basil Bourque
5

Dies beantwortet nicht die genaue Frage, aber es lohnt sich dennoch zu überlegen, ob eine Karte überhaupt zurückgegeben werden sollte. Wenn die Karte unveränderlich ist, basiert die primäre Methode, die bereitgestellt werden soll, auf get (key):

public Integer fooOf(String key) {
    return map.get(key);
}

Dies macht die API viel enger. Wenn tatsächlich eine Karte erforderlich ist, kann dies dem Client der API überlassen werden, indem ein Strom von Einträgen bereitgestellt wird:

public Stream<Map.Entry<String, Integer>> foos() {
    map.entrySet().stream()
}

Anschließend kann der Client nach Bedarf eine eigene unveränderliche oder veränderbare Karte erstellen oder die Einträge zu einer eigenen Karte hinzufügen. Wenn der Client wissen muss, ob der Wert vorhanden ist, kann stattdessen optional zurückgegeben werden:

public Optional<Integer> fooOf(String key) {
    return Optional.ofNullable(map.get(key));
}
Solubris
quelle
-1

Unveränderliche Karte ist eine Art Karte. Das Verlassen des Rückgabetyps der Karte ist also in Ordnung.

Um sicherzustellen, dass die Benutzer das zurückgegebene Objekt nicht ändern, kann die Dokumentation der Methode die Merkmale des zurückgegebenen Objekts beschreiben.

Toro
quelle
-1

Dies ist wohl Ansichtssache, aber die bessere Idee ist hier, eine Schnittstelle für die Kartenklasse zu verwenden. Diese Schnittstelle muss nicht explizit angeben, dass sie unveränderlich ist, aber die Nachricht ist dieselbe, wenn Sie keine der Setter-Methoden der übergeordneten Klasse in der Schnittstelle verfügbar machen.

Schauen Sie sich den folgenden Artikel an:

Andy Gibson

WSimpson
quelle
1
Unnötige Verpackungen gelten als schädlich.
Djechlin
Sie haben Recht, ich würde auch hier keinen Wrapper verwenden, da die Schnittstelle ausreichend ist.
WSimpson
Ich habe das Gefühl, wenn dies das Richtige wäre, würde ImmutableMap bereits ein ImmutableMapInterface implementieren, aber ich verstehe das technisch nicht.
Djechlin
Der Punkt ist nicht das, was die Guava-Entwickler beabsichtigt haben, sondern die Tatsache, dass dieser Entwickler die Architektur kapseln und die Verwendung von ImmutableMap nicht offenlegen möchte. Eine Möglichkeit, dies zu tun, wäre die Verwendung einer Schnittstelle. Wie Sie bereits erwähnt haben, ist die Verwendung eines Wrappers nicht erforderlich und wird daher nicht empfohlen. Die Rückgabe der Karte ist unerwünscht, da sie irreführend ist. Wenn das Ziel die Kapselung ist, ist eine Schnittstelle die beste Option.
WSimpson
Der Nachteil der Rückgabe von Daten, die die MapSchnittstelle nicht erweitern (oder implementieren) , besteht darin, dass Sie sie nicht an eine API-Funktion übergeben können, die eine erwartet, Mapohne sie zu übertragen. Dies schließt alle Arten von Kopierkonstruktoren anderer Arten von Karten ein. Wenn die Veränderlichkeit nur durch die Benutzeroberfläche "verborgen" wird, können Ihre Benutzer dies außerdem jederzeit umgehen, indem Sie Ihren Code auf diese Weise umwandeln und brechen.
Hulk