Czy lepiej jest zwrócić ImmutableMap czy Map?

90

Powiedzmy, że piszę metodę, która powinna zwrócić Map . Na przykład:

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

Po chwili zastanowienia zdecydowałem, że nie ma powodu, aby modyfikować tę mapę po jej utworzeniu. Dlatego chciałbym zwrócić ImmutableMap .

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

Czy powinienem pozostawić zwracany typ jako mapę ogólną, czy powinienem określić, że zwracam ImmutableMap?

Z jednej strony, właśnie po to stworzono interfejsy; aby ukryć szczegóły implementacji.
Z drugiej strony, jeśli zostawię to w ten sposób, inni programiści mogą przegapić fakt, że ten obiekt jest niezmienny. Dlatego nie osiągnę głównego celu, jakim są niezmienne obiekty; aby uczynić kod bardziej przejrzystym, minimalizując liczbę obiektów, które mogą się zmieniać. Co gorsza, po chwili ktoś może spróbować zmienić ten obiekt, a to spowoduje błąd w czasie wykonywania (kompilator nie ostrzeże o tym).

java design-patterns immutable-collections

— AMT
źródło

2

Podejrzewam, że ktoś w końcu oznaczy to pytanie jako zbyt otwarte dla argumentów. Czy możesz zadać bardziej szczegółowe pytania zamiast „WDYT”? i "czy to w porządku ...?" Na przykład: „Czy są jakieś problemy lub kwestie związane z zwracaniem zwykłej mapy?” i "jakie istnieją alternatywy?"

— popiół

Co się stanie, jeśli później zdecydujesz, że powinna ona faktycznie zwrócić zmienną mapę?

— user253751

3

@immibis lol, czy lista w tej sprawie?

— djechlin

3

Czy naprawdę chcesz wstawić zależność Guava do swojego API? Nie będziesz w stanie tego zmienić bez zerwania wstecznej kompatybilności.

— user2357112 obsługuje Monikę

1

Myślę, że pytanie jest zbyt pobieżne i brakuje wielu ważnych szczegółów. Na przykład, czy masz już guawę jako (widoczną) zależność. Nawet jeśli już to masz, fragmenty kodu są pesudocode i nie przekazują tego, co tak naprawdę chcesz osiągnąć. Na pewno byś nie napisał return ImmutableMap.of(somethingElse). Zamiast tego należy zapisać ImmutableMap.of(somethingElse)jako pole i zwrócić tylko to pole. Wszystko to wpływa na projekt tutaj.

— Marco13

70

Jeśli piszesz publicznie dostępny interfejs API i niezmienność jest ważnym aspektem twojego projektu, zdecydowanie przedstawiłbym to wyraźnie, albo mając nazwę metody wyraźnie oznaczającą, że zwrócona mapa będzie niezmienna, albo zwracając konkretny typ Mapa. Wspominanie o tym w javadoc moim zdaniem nie wystarczy.

Ponieważ najwyraźniej używasz implementacji Guava, spojrzałem na dokument i jest to klasa abstrakcyjna, więc daje ci trochę elastyczności w rzeczywistym, konkretnym typie.
Jeśli piszesz wewnętrzne narzędzie / bibliotekę, o wiele bardziej akceptowalne jest zwrócenie zwykłego pliku Map. Ludzie będą wiedzieli o wewnętrznych elementach kodu, do którego dzwonią, lub przynajmniej będą mieli do niego łatwy dostęp.

Mój wniosek byłby taki, że wyraźne jest dobre, nie pozostawiaj rzeczy przypadkowi.

— Dici
źródło

1

Jeden dodatkowy interfejs, jedna dodatkowa klasa abstrakcyjna i jedna klasa, która rozszerza tę klasę abstrakcyjną. Jest to zbyt kłopotliwe jak na tak prostą rzecz, o jaką pyta OP, czyli jeśli metoda powinna zwrócić Maptyp interfejsu lub ImmutableMaptyp implementacji.

— fps

20

Jeśli odnosisz się ImmutableMapdo guawy, rada zespołu Guava jest taka, aby rozważyć ImmutableMapinterfejs z gwarancjami semantycznymi i zwrócić ten typ bezpośrednio.

— Louis Wasserman

1

@LouisWasserman mm, nie widziałem, że pytanie było podaniem linku do implementacji Guava. W takim razie usunę fragment, dziękuję

— Dici

3

Zgadzam się z Louisem, jeśli zamierzasz zwrócić mapę będącą niezmiennym obiektem, upewnij się, że obiekt zwracany jest tego typu najlepiej. Jeśli z jakiegoś powodu chcesz ukryć fakt, że jest to niezmienny typ, zdefiniuj własny interfejs. Pozostawienie tego jako mapy jest mylące.

— WSimpson

1

@WSimpson Myślę, że to właśnie mówi moja odpowiedź. Louis mówił o jakimś fragmencie, który dodałem, ale usunąłem go.

— Dici

38

Powinieneś mieć ImmutableMapjako swój typ zwrotu. Mapzawiera metody, które nie są obsługiwane przez implementację ImmutableMap(np. put) i są oznaczone @deprecatedw ImmutableMap.

Używanie przestarzałych metod spowoduje ostrzeżenie kompilatora, a większość IDE ostrzeże, gdy ludzie spróbują użyć przestarzałych metod.

To zaawansowane ostrzeżenie jest lepsze niż stosowanie wyjątków środowiska wykonawczego jako pierwszej wskazówki, że coś jest nie tak.

— Synesso
źródło

1

To moim zdaniem najbardziej sensowna odpowiedź. Interfejs Map jest kontraktem, a ImmutableMap wyraźnie narusza jego ważną część. Używanie Map jako typu zwracanego w tym przypadku nie ma sensu.

— TonioElGringo

@TonioElGringo co Collections.immutableMapwtedy?

— OrangeDog

@TonioElGringo Zwróć uwagę, że metody, których ImmutableMap nie implementują, są wyraźnie oznaczone jako opcjonalne w dokumentacji interfejsu docs.oracle.com/javase/7/docs/api/java/util/… . Dlatego myślę, że nadal może mieć sens zwracanie mapy (z odpowiednimi javadocami). Mimo to zdecydowałem się jawnie zwrócić ImmutableMap z powodów omówionych powyżej.

— AMT

3

@TonioElGringo nie narusza niczego, te metody są opcjonalne. Podobnie jak w przypadku List, nie ma gwarancji, że List::addjest ważna. Spróbuj Arrays.asList("a", "b").add("c");. Nic nie wskazuje na to, że zakończy się niepowodzeniem w czasie wykonywania, ale tak się dzieje. Przynajmniej Guava cię ostrzega.

— njzk2

14

Z drugiej strony, jeśli zostawię to w ten sposób, inni programiści mogą przegapić fakt, że ten obiekt jest niezmienny.

Należy o tym wspomnieć w pliku javadocs. Wiesz, programiści je czytają.

Dlatego nie osiągnę głównego celu, jakim są niezmienne obiekty; aby uczynić kod bardziej przejrzystym, minimalizując liczbę obiektów, które mogą się zmieniać. Co gorsza, po chwili ktoś może spróbować zmienić ten obiekt, a to spowoduje błąd w czasie wykonywania (kompilator nie ostrzeże o tym).

Żaden programista nie publikuje nieprzetestowanego kodu. A kiedy to przetestuje, zostaje wyrzucony wyjątek, w którym nie tylko widzi przyczynę, ale także plik i wiersz, w którym próbował napisać na niezmiennej mapie.

Zwróć jednak uwagę, że tylko on Mapsam będzie niezmienny, a nie obiekty, które zawiera.

— tkausl
źródło

10

„Żaden programista nie publikuje nieprzetestowanego kodu”. Chcesz na to postawić? Byłoby znacznie mniej (przypuszczam) błędów w publicznym oprogramowaniu, gdyby to zdanie było prawdziwe. Nie byłbym nawet pewien, czy stwierdzenie „większość programistów…” byłoby prawdą. I tak, to rozczarowujące.

— Tom

1

Okej, Tom ma rację, nie jestem pewien, co próbujesz powiedzieć, ale to, co faktycznie napisałeś, jest ewidentnie fałszywe. (Również: popychanie do włączenia płci)

— djechlin

@Tom Chyba przegapiłeś sarkazm w tej odpowiedzi

— Kapitan Fogetti

10

jeśli zostawię to w ten sposób, inni programiści mogą przegapić fakt, że ten obiekt jest niezmienny

To prawda, ale inni programiści powinni przetestować swój kod i upewnić się, że jest on objęty ochroną.

Niemniej jednak masz jeszcze 2 opcje rozwiązania tego problemu:

Użyj Javadoc
```
@return a immutable map
```

Wybierz opisową nazwę metody

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

Dla konkretnego przypadku użycia możesz nawet lepiej nazwać metody. Na przykład

public Map<String, Integer> getUnmodifiableCountByWords()

Co jeszcze możesz zrobić?!

Możesz zwrócić

Kopiuj
```
private Map<String, Integer> myMap;

public Map<String, Integer> foo() {
  return new HashMap<String, Integer>(myMap);
}
```
To podejście powinno być stosowane, jeśli spodziewasz się, że wielu klientów będzie modyfikować mapę i jeśli mapa zawiera tylko kilka wpisów.
CopyOnWriteMap

kolekcje copy on write są zwykle używane, gdy masz do czynienia ze
współbieżnością. Ale koncepcja pomoże ci również w twojej sytuacji, ponieważ CopyOnWriteMap tworzy kopię wewnętrznej struktury danych podczas operacji mutacyjnej (np. Dodaj, usuń).

W takim przypadku potrzebujesz cienkiej otoki wokół mapy, która deleguje wszystkie wywołania metod do podstawowej mapy, z wyjątkiem operacji mutacyjnych. W przypadku wywołania operacji mutacyjnej tworzy ona kopię mapy bazowej, a wszystkie dalsze wywołania zostaną delegowane do tej kopii.

Takie podejście należy zastosować, jeśli spodziewasz się, że niektórzy klienci zmodyfikują mapę.

Niestety java nie ma takiego pliku CopyOnWriteMap. Ale możesz znaleźć firmę zewnętrzną lub wdrożyć ją samodzielnie.

W końcu powinieneś pamiętać, że elementy na twojej mapie mogą nadal być zmienne.

— René Link
źródło

8

Zdecydowanie zwróć ImmutableMap, z uzasadnieniem:

Podpis metody (w tym typ zwrotu) powinien być samodokumentujący. Komentarze są jak obsługa klienta: jeśli Twoi klienci muszą na nich polegać, oznacza to, że Twój podstawowy produkt jest uszkodzony.
To, czy coś jest interfejsem, czy klasą, ma znaczenie tylko podczas rozszerzania lub implementowania. Biorąc pod uwagę instancję (obiekt), przez 99% czasu kod klienta nie będzie wiedział lub nie dbał o to, czy coś jest interfejsem, czy klasą. Z początku zakładałem, że ImmutableMap to interfejs. Dopiero po kliknięciu linku zdałem sobie sprawę, że to klasa.

— Benito Ciaro
źródło

5

To zależy od samej klasy. Guawa ImmutableMapnie ma być niezmiennym widokiem na zmienną klasę. Jeśli Twoja klasa jest niezmienna i ma jakąś strukturę, która jest w zasadzie typu an ImmutableMap, utwórz typ zwracany ImmutableMap. Jeśli jednak twoja klasa jest zmienna, nie rób tego. Jeśli masz to:

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

Guawa za każdym razem skopiuje mapę. To jest powolne. Ale jeśli internalMapjuż był ImmutableMap, to jest w porządku.

Jeśli nie ograniczysz swojej klasy do powrotu ImmutableMap, możesz zamiast tego wrócić w następujący Collections.unmodifiableMapsposób:

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

Zauważ, że jest to niezmienny widok na mapę. Jeśli się internalMapzmieni, tak samo będzie z buforowaną kopią Collections.unmodifiableMap(internalMap). Jednak nadal wolę to dla getterów.

— Justin
źródło

1

To są dobre punkty, ale dla kompletności podoba mi się ta odpowiedź, która wyjaśnia, że unmodifiableMapjest niemodyfikowalny tylko przez posiadacza odniesienia - jest to widok, a posiadacz mapy podkładowej może modyfikować mapę podkładową, a następnie widok się zmienia. To ważna kwestia.

— davidbak

@Jak skomentował davidback, bądź bardzo ostrożny przy używaniu Collections.unmodifiableMap. Ta metoda zwraca widok oryginalnej mapy zamiast tworzenia oddzielnego, nowego obiektu mapy. Tak więc każdy inny kod odwołujący się do oryginalnej mapy może ją zmodyfikować, a następnie posiadacz rzekomo niemodyfikowalnej mapy dowiaduje się na własnej skórze, że mapa rzeczywiście może być modyfikowana pod nimi. Sam zostałem ugryziony przez ten fakt. Nauczyłem się albo zwracać kopię mapy, albo używać guawy ImmutableMap.

— Basil Bourque

5

Nie jest to dokładna odpowiedź na pytanie, ale nadal warto się zastanowić, czy mapa w ogóle powinna zostać zwrócona. Jeśli mapa jest niezmienna, podstawowa metoda, która ma być dostarczona, opiera się na get (klucz):

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

To sprawia, że API jest znacznie mocniejsze. Jeśli mapa jest rzeczywiście wymagana, można to pozostawić klientowi API, dostarczając strumień wpisów:

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

Następnie klient może utworzyć własną niezmienną lub zmienną mapę według potrzeb lub dodać wpisy do własnej mapy. Jeśli klient chce wiedzieć, czy wartość istnieje, można zamiast tego zwrócić opcjonalne:

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

— Solubris
źródło

-1

Niezmienna mapa to rodzaj mapy. Więc pozostawienie zwracanego typu mapy jest w porządku.

Aby upewnić się, że użytkownicy nie modyfikują zwracanego obiektu, dokumentacja metody może opisywać cechy zwracanego obiektu.

— toro
źródło

-1

Jest to prawdopodobnie kwestia opinii, ale lepszym pomysłem jest tutaj użycie interfejsu dla klasy map. Ten interfejs nie musi wyraźnie mówić, że jest niezmienny, ale komunikat będzie taki sam, jeśli nie ujawnisz żadnej z metod ustawiających klasy nadrzędnej w interfejsie.

Spójrz na następujący artykuł:

andy gibson

— WSimpson
źródło

1

Niepotrzebne opakowania uważane za szkodliwe.

— djechlin

Masz rację, tutaj też nie użyłbym opakowania, ponieważ interfejs jest wystarczający.

— WSimpson

Mam wrażenie, że gdyby było to właściwe, ImmutableMap zaimplementowałby już ImmutableMapInterface, ale nie rozumiem tego technicznie.

— djechlin

Nie chodzi o to, co zamierzali programiści Guava, ale o fakt, że ten programista chciałby hermetyzować architekturę i nie ujawniać użycia ImmutableMap. Jednym ze sposobów byłoby użycie interfejsu. Używanie opakowania, jak wskazałeś, nie jest konieczne i dlatego nie jest zalecane. Zwracanie mapy jest niepożądane, ponieważ wprowadza w błąd. Wydaje się więc, że jeśli celem jest hermetyzacja, najlepszym rozwiązaniem jest interfejs.

— WSimpson

Wadą zwracania czegoś, co nie rozszerza (ani nie implementuje) Mapinterfejsu jest to, że nie można go przekazać do żadnej funkcji API, która oczekuje a Mapbez rzutowania. Obejmuje to wszelkiego rodzaju konstruktory kopiujące innych typów map. Oprócz tego, jeśli zmienność jest tylko „ukryta” przez interfejs, użytkownicy mogą zawsze obejść ten problem, przesyłając i niszcząc kod w ten sposób.

— Hulk