Co jest nie tak z tym kodem?

Odpowiedz Nowy wątek
2017-08-11 15:14
0

Hej,

Poniższy fragment kodu został odrzucony na code review. Czy możecie mi powiedzieć co jest z nim nie tak? Komentarz recenzenta na razie przemilczę.

    private User getUser(long userId) {
        User user = userRepository.findById(userId);
        if (user == null) {
            throw new ResourceNotFoundException("User with id: " + userId + " not found.");
        }
        return user;
    }

Pozostało 580 znaków

2017-08-11 15:33
0

A to w ogóle nie jest Unhandled exception?

Pozostało 580 znaków

2017-08-11 15:37
0

Jest to custom RuntimeException.

Pozostało 580 znaków

2017-08-11 15:37
Zakręcony Szczur
0

Zakładam że jest to metoda z jakiegoś @Controllera więc z tego co czytałem na forum nie może zwracać encji - ale mogę się mylić :D

Pozostało 580 znaków

2017-08-11 15:42
0

Być może musisz zwrócić jakieś DTO, zamiast bezpośrednio obiektu z repozytorium?


Wenn ist das Nunstück git und Slotermeyer? Ja! Beiherhund das Oder die Flipperwaldt gersput!

Pozostało 580 znaków

2017-08-11 15:46
0

Wydaje mi się, że istotny jest kontekst w jakim ta metoda jest tworzona. Gdzie ją tworzysz? Btw. Jaki komentarz dał ? :)

Edit: Sokoloki. Czy tu nie chodzi o prywatny getter? Dane pobrane z bazy prawdopodobnie chcesz przekazać dalej.
Edit2: Optionala możesz użyć.

Swoją droga fajny code review jak Ci nie mówią co jest źle

edytowany 4x, ostatnio: Prędki_Lopez, 2017-08-11 15:51
komentarz jest, ale ujawnię go później - pisałem o tym w pierwszej wiadomości - xLatency 2017-08-11 16:17

Pozostało 580 znaków

2017-08-11 16:06
0

po co w ogole ta metoda zamiast po prostu uzywac userRepository.findById(userId) :)

Pokaż pozostałe 3 komentarze
jakby co to nie bylo pytania, cos mi się pomerdało, zmęczony już - Prędki_Lopez 2017-08-11 16:30
spoko, ale pewnie i tak bym nie odpowiedziala bo nie znam zbytnio wzorcow zwiazanych z repozytoriami - katelx 2017-08-11 16:35
Na moje to Optionala zwrócić i go sprawdzać potem czy jest pusty, ale są takie momenty chyba kiedy chcemy rzucić po prostu wyjątek jesli nie znajdziemy wymaganych danych w bazie. - Prędki_Lopez 2017-08-11 16:40
nie lubie wyjatkow poza rzeczywiscie powaznymi sytuacjami bo szybko sie robi z tego instrukcja sterujaca. w wypadku takim jak tutaj zgadzam sie co do optionala + nazwe metody na tryFindById zeby od razu bylo widac ze spodziewamy sie faila - katelx 2017-08-11 16:45
Kupę lat temu stosowałem takie nazewnictwo: getById() / checkById() - wywal wyjątek jak nie ma, findById() - zwróć null. W szczególności jedno wołało drugie, ale obsługa nulla była w jednym miejscu. - vpiotr 2017-08-11 17:24

Pozostało 580 znaków

2017-08-11 16:27
4

@xLatency: kurde żyjemy w 2017 roku, Java 8 ma 3 lata a dalej się nie nauczyli że z DAO/Repository jak zwracamy encje której może nie byc to zwracamy Optional?
Kurde heloł ludzie Java 9 za kilkadziesiąt dni (mam nadzieje :D ) a dalej nie umiecie into Java 8? oO


Nie pomagam przez PM. Pytania zadaje się na forum.
java 9 juz za kilkaset dni ;) poza tym nie badz swinia, niektorzy nie wyszli ponad jave 7 :( - katelx 2017-08-11 16:30
Und dort liegt der Hund begraben - jarekr000000 2017-08-11 16:52
@jarekr000000: czy dobrze google translator mi tłumaczy "i tu jest pies pogrzebany"? - scibi92 2017-08-11 16:54

Pozostało 580 znaków

2017-08-11 16:46
Krzywy kr
0
scibi92 napisał(a):

@xLatency: kurde żyjemy w 2017 roku, Java 8 ma 3 lata a dalej się nie nauczyli że z DAO/Repository jak zwracamy encje której może nie byc to zwracamy Optional?
Kurde heloł ludzie Java 9 za kilkadziesiąt dni (mam nadzieje :D ) a dalej nie umiecie into Java 8? oO

Czasami wole zwyklego null checka niż przeginanie z Optionalami.

Pozostało 580 znaków

2017-08-11 17:09
2

Problem zależy od kontekstu , ale kod generalnie useless - SpringData od dawna zwracać może Optionale. Wiec po co robić takie nonsensowne przepakowywanie - jak można u źródła naprawić.


Bardzo lubie Singletony, dlatego robię po kilka instancji każdego.
edytowany 2x, ostatnio: jarekr000000, 2017-08-11 17:10

Pozostało 580 znaków

2017-08-11 17:11
0

Zamieniasz nulla na wyjątek. Zarówno nulla jak i wyjątek trzeba obsłużyć w logice, która korzysta z tej metody.
Gdzie widzisz wartość z tej konwersji?

Możliwe, że chodzi o uniknięcie duplikacji kodu, ale oznaczałoby, że w ramach klasy często korzystasz z getUser(). Jeśli tak, to może klasa powinna odstawać na wejściu Usera, a nie pobierać go sobie z repozytorium? Nie wiedzę całej klasy, więc bez kontekstu ciężko ocenić czy taka konstrukcja ma jakiekolwiek zalety.

Pozostało 580 znaków

Odpowiedz
Liczba odpowiedzi na stronę

1 użytkowników online, w tym zalogowanych: 0, gości: 1, botów: 0