Jak mogę uniknąć powtarzania kodu inicjującego skrót mapy?

27

Każdy klient ma identyfikator i wiele faktur, wraz z datami, przechowywanymi jako Hashmap klientów według identyfikatorów, skrótów faktur według dat:

HashMap<LocalDateTime, Invoice> allInvoices = allInvoicesAllClients.get(id);

if(allInvoices!=null){
    allInvoices.put(date, invoice);      //<---REPEATED CODE
}else{
    allInvoices = new HashMap<>();
    allInvoices.put(date, invoice);      //<---REPEATED CODE
    allInvoicesAllClients.put(id, allInvoices);
}

Wydaje się, że rozwiązaniem Java jest użycie getOrDefault:

HashMap<LocalDateTime, Invoice> allInvoices = allInvoicesAllClients.getOrDefault(
    id,
    new HashMap<LocalDateTime, Invoice> (){{  put(date, invoice); }}
);

Ale jeśli get nie ma wartości zerowej, nadal chcę wykonać polecenie put (data, faktura), a także dodanie danych do „allInvoicesAllClients” jest nadal potrzebne. Więc to niewiele pomaga.

Hernán Eche
źródło
Jeśli nie możesz zagwarantować wyjątkowości klucza, najlepiej założyć, że mapa dodatkowa ma wartość Lista <Faktura> zamiast tylko Faktury.
Ryan

Odpowiedzi:

39

Jest to doskonały przypadek użycia dla Map#computeIfAbsent. Twój fragment kodu jest zasadniczo równoważny z:

allInvoicesAllClients.computeIfAbsent(id, key -> new HashMap<>()).put(date, invoice);

Jeśli idnie jest obecny jako klucz allInvoicesAllClients, utworzy mapowanie z idna nowy HashMapi zwróci nowy HashMap. Jeśli idjest obecny jako klucz, wówczas zwróci istniejący HashMap.

Jacob G.
źródło
1
computeIfAbsent, wykonuje get (id) (lub put następuje po get (id)), więc następna put jest wykonywana w celu poprawienia put pozycji (data), poprawna odpowiedź.
Hernán Eche
allInvoicesAllClients.computeIfAbsent(id, key -> Map.of(date, invoice))
Alexander - Przywróć Monikę
1
@ Alexander-ReinstateMonica Map.oftworzy niemodyfikowalne Map, czego nie jestem pewien OP chce.
Jacob G.
Czy ten kod byłby mniej wydajny niż pierwotnie OP? Pytanie o to, ponieważ nie znam sposobu, w jaki Java obsługuje funkcje lambda.
Zecong Hu
16

computeIfAbsentjest doskonałym rozwiązaniem dla tego konkretnego przypadku. Ogólnie chciałbym zwrócić uwagę na następujące kwestie, ponieważ nikt jeszcze o tym nie wspominał:

„Zewnętrzna” mapa skrótów po prostu przechowuje odniesienie do „wewnętrznej” tablicy skrótów, dzięki czemu można po prostu zmienić kolejność operacji, aby uniknąć powielania kodu:

HashMap<LocalDateTime, Invoice> allInvoices = allInvoicesAllClients.get(id);

if (allInvoices == null) {           
    allInvoices = new HashMap<>();
    allInvoicesAllClients.put(id, allInvoices);
}

allInvoices.put(date, invoice);      // <--- no longer repeated
Heinzi
źródło
Tak robiliśmy przez dziesięciolecia, zanim Java 8 pojawiła się wraz ze swoją wymyślną computeIfAbsent()metodą!
Neil Bartlett,
1
Nadal używam tego podejścia dzisiaj w językach, w których implementacja mapy nie zapewnia jednej metody get-or-put-and-return-if-nieobecny. Warto wspomnieć, że wciąż może to być najlepsze rozwiązanie w innych językach, nawet jeśli pytanie to jest specjalnie oznaczone w Javie 8.
Quinn Mortimer
11

Prawie nigdy nie powinieneś używać inicjalizacji mapy „podwójny nawias klamrowy”.

{{  put(date, invoice); }}

W takim przypadku powinieneś użyć computeIfAbsent

allInvoicesAllClients.computeIfAbsent(id, (k) -> new HashMap<>())
                     .put(date, allInvoices);

Jeśli nie ma mapy dla tego identyfikatora, wstawisz ją. Wynikiem będzie istniejąca lub obliczona mapa. Możesz następnie putprzedmioty na tej mapie z gwarancją, że nie będzie ona zerowa.

Michał
źródło
1
Nie wiem, kto głosował, a nie ja, być może kod jednoliniowy myli wszystkie wprowadzone fakturyAllClients, ponieważ używasz identyfikatora zamiast daty, ja go
edytuję
1
@ HernánEche Ah. Mój błąd. Dzięki. Tak, idgotowe jest również gotowe. Jeśli chcesz, możesz traktować to computeIfAbsentjako warunkowe. I zwraca również wartość
Michael
Prawie nigdy nie powinieneś używać inicjalizacji mapy„ podwójny nawias klamrowy ”. „ Dlaczego? (Nie wątpię, że masz rację; proszę z prawdziwej ciekawości.)
Heinzi
1
@Heinzi Ponieważ tworzy anonimową klasę wewnętrzną. To zawiera odniesienie do klasy, która ją zadeklarowała, co jeśli ujawnisz mapę (np. Za pomocą gettera), zapobiegnie gromadzeniu śmieci przez klasę otaczającą. Uważam też, że może to być mylące dla osób mniej zaznajomionych z Javą; bloki inicjalizujące prawie nigdy nie są używane, a pisanie go w ten sposób sprawia, że ​​wygląda na to, że {{ }}ma specjalne znaczenie, czego nie ma.
Michael
1
@Michael: Ma sens, dzięki. Zupełnie zapomniałem, że anonimowe klasy wewnętrzne są zawsze niestatyczne (nawet jeśli nie muszą).
Heinzi
5

Jest to dłuższe niż inne odpowiedzi, ale znacznie bardziej czytelne:

if(!allInvoicesAllClients.containsKey(id))
    allInvoicesAllClients.put(id, new HashMap<LocalDateTime, Invoice>());

allInvoicesAllClients.get(id).put(date, invoice);
Wilk
źródło
3
Może to działać w przypadku HashMap, ale ogólne podejście nie jest optymalne. Jeśli byłyby to ConcurrentHashMaps, operacje te nie byłyby atomowe. W takim przypadku sprawdzenie, a następnie działanie doprowadzi do warunków wyścigu. W każdym razie poproszono hejterów.
Michael
0

Robisz tutaj dwie oddzielne rzeczy: upewnienie się, że HashMapistnieje i dodanie do niej nowego wpisu.

Istniejący kod upewnia się, że najpierw wstawisz nowy element przed zarejestrowaniem mapy skrótu, ale nie jest to konieczne, ponieważ tutaj HashMapnie zależy na zamówieniu. Żaden wariant nie jest bezpieczny dla wątków, więc nic nie tracisz.

Tak jak sugerował @Heinzi, możesz po prostu podzielić te dwa kroki.

Co chciałbym także zrobić to odciążyć stworzenia HashMapdo allInvoicesAllClientsobiektu, więc getmetoda ta nie może powrócić null.

Zmniejsza to również możliwość wyścigów między osobnymi wątkami, które mogłyby zarówno pobrać nullwskaźniki, geta następnie zdecydować się putna nowy HashMapz jednym wpisem - drugi putprawdopodobnie odrzuciłby pierwszy, tracąc Invoiceobiekt.

Simon Richter
źródło