Zwraca wartość logiczną set.add () w warunkowym?

Operator add klasy set zwraca wartość logiczną, która jest prawdą, jeśli element (który ma zostać dodany) jeszcze nie istniał, a fałsz w przeciwnym razie. Pisze

if (set.add(entry)) {
    //do some more stuff
}

uważany za dobry styl pod względem pisania czystego kodu? Zastanawiam się, skoro robicie dwie rzeczy naraz. 1) dodanie elementu i 2) sprawdzenie, czy element istniał.

Tak to jest.

Zazwyczaj operacja zwraca wartość logiczną, ponieważ można jej użyć do podejmowania decyzji, tj. W obrębie ifkonstrukcji. Jedynym innym sposobem, w jaki mógłbyś zrealizować ten potencjał, byłoby przechowywanie wartości zwracanej w zmiennej, a następnie natychmiastowe użycie jej w zmiennej if, która jest po prostu głupia i na pewno nie jest lepsza od pisania if(operation()) { ... }. Więc po prostu idź i zrób to, nie będziemy cię oceniać (za to).

Powiedziałbym, że nie jest tak czysty, jak to możliwe, ponieważ zmusza opiekuna albo już wie, albo sprawdza, co oznacza wartość zwracana. Czy to oznacza, że ​​wartość już istniała, jeszcze nie istniała, została pomyślnie wstawiona? Jeśli nie użyjesz go dużo, nie będziesz wiedział, a nawet jeśli to zrobisz, będzie to o wiele więcej obciążenia psychicznego.

Wolałbym następujące:

boolean added = set.add(entry);

if (added) {
    //do some more stuff
}

Tak, nieco bardziej szczegółowe, ale kompilator powinien generować dokładnie ten sam kod bajtowy, a nawet ludzie, którzy nie używali zestawów Java od lat, mogą podążać za logiką, nie szukając niczego.

Jeśli prawda oznacza sukces, to jest to dobry, czytelny kod.

Istnieje powszechna konwencja, że ​​funkcja lub metoda zwraca true (lub coś, co daje wartość true) po sukcesie. Tak długo, jak podąża za tym twój kod, myślę, że umieszczenie metody w warunku jest w porządku.

Moim zdaniem taki kod jest niepotrzebnie zaśmiecony:

boolean frobulate_succeeded = thing.frobulate();

if (frobulate_succeeded) {
    ...
}

Czujesz się, jakbyś się powtarzał.

Pytanie jest jednak dwuznaczne co do znaczenia zwracanej wartości. Mówisz „boolean, który wskazuje, czy dodany element już istniał”, co może sugerować, że prawda oznacza, że ​​element istniał (i dodawanie nie nastąpiło). W takim przypadku idealnie zmieniłbym zachowanie zwrotne metody na bardziej konwencjonalne. Jeśli nie jest to możliwe, dodałbym dodatkową zmienną pośrednią, która pozwala wyraźnie oznaczyć wynik w kodzie (jak sugerują inni).

Powiedziałbym, że jest bardzo podobny do C. Przez większość czasu wolałbym mieć opisowo nazwaną zmienną dla wyniku mutacji i żadna mutacja nie zachodzi w ifstanie.

Kompilator wyeliminuje tę zmienną, jeśli zostanie natychmiast ponownie wykorzystana. Ludziom łatwiej będzie czytać źródło; dla mnie jest to ważniejsze.

Gdyby ktoś musiał rozszerzyć warunek poprzez dodanie klauzuli and/ ordo warunku if, może .add()w niektórych przypadkach nie zadzwonić z powodu oceny zwarcia. O ile nie przewiduje się zwarcia, może to być błąd.

Wygląda na to, że Twój kod przerywa rozdzielanie zapytań poleceń . Jest to omówione w książce Clean Code i wideo Struktury funkcji . Więc z perspektywy Clean Code myślę, że nie jest uważany za dobry styl.

„Czysty kod jest prosty i bezpośredni. Czysty kod czyta się jak dobrze napisana proza. Czysty kod nigdy nie przesłania intencji projektanta, ale jest pełen wyraźnych abstrakcji i prostych linii kontroli.

- Grady Booch autor Object Oriented Analysis and Design with Applications ”

Dla mnie cel twojego kodu jest niejasny. Czy oba są wykonywane, gdy pozycja jest dodawana, czy też już istnieje? Co add()zwraca przedmiot? Kod błędu?