Wątek przeniesiony 2019-03-30 13:36 z przez Patryk27.

Prośba o Code Review

Odpowiedz Nowy wątek
2019-03-30 12:37
0

Cześć. Piszę do was z prośbą o ocenę kodu mojej aplikacji. Pisałem ją przez 4 wieczory i 3 dni od zera. Jest to mój pierwszy "większy" projekt w Springu i zdaję sobie sprawę, że to tylko prosty CRUD bez logiki. Pisząc go chciałem podsumować swoją wiedzę przed przejściem do nauki REST`ów i przy okazji stworzyć jakiś projekt do portfolio, które chciałbym ewentualnie kiedyś wykorzystać do znalezienia pierwszego stażu wakacyjnego. Planuję w najbliższym wolnym czasie popracować nad Security ( dodawanie użytkowników, edycja ról itp. ). Prosiłbym o skupienie się na samym backendzie, ponieważ frontend u mnie raczkuje. Z góry dziękuję za pomoc, rady, konstruktywną krytykę i poświęcony czas.

Link do repozytorium : https://github.com/DSniatecki/YourFleetManager

Pozostało 580 znaków

2019-04-16 18:30
1

Co ty w ogóle testujesz.? https://github.com/DSniatecki[...]rs/CarControllerUnitTest.java
Ja tam widzę mocka który operuje na drugim mocku.

Po co ci ten interfejs ? https://github.com/DSniatecki[...]yourfleetmanager/services/car

To nie są żadne obiekty domenowe zwane potocznie "encjami". No chyba, że twoją domeną jest relacyjna baza danych.
https://github.com/DSniatecki[...]cki/yourfleetmanager/entities

Nie da się tego jakoś lepiej napisać w Javie ?
https://github.com/DSniatecki[...]ers/car/CarPartialMapper.java

No i całość to oczywiście przepyszna lazania.


Unhandled Exception: System.MissingMethodException: Constructor on type 'System.Exception' not found.

Pozostało 580 znaków

2019-04-16 19:11
0
Gworys napisał(a):

Co ty w ogóle testujesz.? https://github.com/DSniatecki[...]rs/CarControllerUnitTest.java
Ja tam widzę mocka który operuje na drugim mocku.

  • Prosty test jednostkowy, czy poprawnie zachodzi proces serializacji do JSON`a i czy jest dobry status,

Po co ci ten interfejs ? https://github.com/DSniatecki[...]yourfleetmanager/services/car

  • Staram się korzystać z Dependency Injection do interfesjów. Wydaje mi się, że jest do dobra praktyka.

To nie są żadne obiekty domenowe zwane potocznie "encjami". No chyba, że twoją domeną jest relacyjna baza danych.
https://github.com/DSniatecki[...]cki/yourfleetmanager/entities

  • To jak to poprawnie nazwać ? Założyłem, że skoro coś jest oznaczone przez adnotacje @Entity to jest to encja. Korzystam z relacyjnej bazy danych, ale nie jest to moja domena.

Nie da się tego jakoś lepiej napisać w Javie ?
https://github.com/DSniatecki[...]ers/car/CarPartialMapper.java

  • Tego akurat wydaje mi się, że nie. Jest to specjalnie tak napisane by przypisywało tylko co nie jest nullem. Dzięki temu jak przekaże tylko część parametrów w JSON`ie to tylko ta część zostanie zmieniona. Taki był mój cel.

No i całość to oczywiście przepyszna lazania.

  • Dlaczego? Wydaje mi się, że wszystko idzie według dobrego schematu i na siebie nie nachodzi. Controller odpiera DTOSy > Service mapuje > Repo zapisuje. Jest wiele DTOsów, co mogło popsuć czytelność, ale schemat jest chyba w miarę dobry.

Ciągle się uczę i dzisiaj użyłbym projekcji, trochę pozmieniał te testy, częściej tworzył i korzystał z builderów. Czytelność jest dla mnie zazwyczaj priorytetem. Jeśli coś jest źle to chętnie to poprawię, ale chciałbym zrozumieć ten błąd. Trochę nie rozumiem tego negatywnego podejścia, ale i tak dzięki. Super byłoby gdybyś wytłumaczył trochę bardziej te błędy, ale nic nie wymagam :)

edytowany 4x, ostatnio: DamianSn, 2019-04-16 19:48

Pozostało 580 znaków

2019-04-16 21:38
2

DI nie polega na tworzeniu interface a na wrzucaniu zależności z zewnątrz (przez konstruktor czy jak tam zwał). Jak masz mieć tylko jedną implementacje to nie ma sensu mieć do tego interface


Spring? Ja tam wole mieć kontrole nad kodem ᕙ(ꔢ)ᕗ
Źle to opisałem, ale dzięki za radę z tym, że nie warto przy jednej klasie :) - DamianSn 2019-04-16 21:40

Pozostało 580 znaków

2019-04-17 06:58
1

Prosty test jednostkowy, czy poprawnie zachodzi proces serializacji do JSON`a i czy jest dobry status,

Testowanie mocka z mockiem wydaje mi się tak samo bez sensu, jak rzucanie wyjątku, kiedy Optional jest "non'em".

     private void checkIsNotPresent(Optional<Company> optional, Long id){
        if(!optional.isPresent()){
            throw new ResourceNotFoundException("Company[id:" + id +"] was not found.");
        }

To jak to poprawnie nazwać ? Założyłem, że skoro coś jest oznaczone przez adnotacje @Entity to jest to encja. Korzystam z relacyjnej bazy danych, ale nie jest to moja domena.

A no fakt jest tam logika biznesowa. :)

    public void addCar(Car car){
        if(this.cars == null) this.cars = new ArrayList<>();
        car.setDepartment(this);
        this.cars.add(car);
    }

Wiec nie można powiedzieć, że nie jest to DM.

Ja bym wolał po prostu operować na presistance objects z DAO. No, chyba że ta warstwa domenowa to jakaś inwestycja na przyszłość.

Tego akurat wydaje mi się, że nie. Jest to specjalnie tak napisane by przypisywało tylko co nie jest nullem. Dzięki temu jak przekaże tylko część parametrów w JSON`ie to tylko ta część zostanie zmieniona. Taki był mój cel.

Nie można tam użyć Optionala z 'Map'.?

Dlaczego? Wydaje mi się, że wszystko idzie według dobrego schematu i na siebie nie nachodzi. Controller odpiera DTOSy > Service mapuje > Repo zapisuje. Jest wiele DTOsów, co mogło popsuć czytelność, ale schemat jest chyba w miarę dobry.

Tworzenie modułów per nazwa wzorca lub czegoś, co nawiązuje do używanego języka jak np. "Interfaces" zawsze jest złe.
Najpierw zawsze powinno występować powiązanie logiczne modelu.
np.
Departaments
---DepartamentSrvice
---DepartamentDto
---Cars
------CarsDto
------CarsService

Nie musisz też wszędzie używać nazw 'Service' często zabija to kreatywność i pogarsza czytelność modelu. Czasem lepiej jest napisać to co ta usługa robi.
np. Zamiast BoxService możesz napisać PackingBox.


Unhandled Exception: System.MissingMethodException: Constructor on type 'System.Exception' not found.

Pozostało 580 znaków

2019-04-17 19:48
1

Dzięki za wytłumaczenie i wiele cennych uwag. W wolnym czasie wezmę się za poprawianie tej lazanii :)

edytowany 1x, ostatnio: DamianSn, 2019-04-17 19:49

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