Ocena i pomoc w dalszym działaniu.

0

Witam.
Zacząłem się bawić co nie co w Java, a dokładniej w Spring Framework. Postawiłem sobie za cel stworzenie sklepu internetowego z bazą danych. Napisałem już trochę kodu i chciałem, żebyście go zobaczyli i powiedzieli, gdzie ewentualnie mogę coś poprawić lub mam jakieś niedociągnięcia. Mam chęć również rozbudować jeszcze ten sklep o logowanie i koszyk. Jeśli mielibyście ciekawe i pomocne materiały do tych tematów to byłbym wdzięczny, jeśli podzielilibyście się ze mną. Czekam na wasze opinie.

Link do gita: https://github.com/biernat12/webstore

1
  1. Na pewno nie ma sensu trzymać w repo ustawień od intelliJ
  2. Ten plik jest chyba zbędny - mnelson.txt
  3. W kodzie jest dużo komentarzy - niepotrzebne, bo i tak nic nie wnoszą do kodu.
  4. Nie znam się na javie za bardzo, ale czy jest różnica między spring framework a spring mvc? Posiadam książkę o Spring MVC i przepisany kod jest bardziej zaawansowany niż to repozytorium.
0

1.Klasa ProductServices stanowi jedynie wrapper na repozytorium - czyli jest zbędna.
2.ProductRepository, a nie ProductRepositories (wszak reprezentuje jedno repozytorium, a nie kilka).
3.Nie commituj zakomentowanego kodu.
4.W każdym pliku HTML powtarzasz ten sam ciąg. Co by było, gdybyś miał setkę plików i potrzebę zmiany jakieś drobnostki w sekcji head?

1

Kilka uwag na wyrywki: to, co wytknąłbym w code review.

Przede wszystkim niekonsekwentne nazewnictwo. Np. dlaczego ProductRepositories, skoro obiekt ewidentnie reprezentuje pojedyncze repozytorium?

Czemu jeden package jest nazwany w liczbie mnogiej - models, a inny już w liczbie pojedynczej: controller, mimo że też zawiera więcej niż jeden kontroler?

Czemu getProductByProductId, a nie po prostu getProductById? To się rozumie samo przez się jak dla mnie, że to musi być jego ID.

A jeśli już upieramy się przy "by product id", to zgodnie z tą logiką, dlaczego nie getProductsByProductCategory, tylko raz tak, a raz tak? :)

Czym się różni pojęcie "find" od "get"? Gdy chcemy pobrać wszystkie produkty, używamy metody findAll, ale gdy stosujemy jakieś kryterium (np. kategorię), wtedy nazwy metod zaczynają się od get. Niby drobiazg, ale stosowanie synonimów w kodzie powoduje szum informacyjny.

Odwołanie do this. są potrzebne, gdy zachodzi problem wieloznaczności - np. rozróżnienia między parametrem metody a polem klasy.

Tu np. nie ma jednak takiego problemu:

    @Override
    public List<Product> findAll() {
        return this.productRepositories.findAll();
    }

więc this jest raczej zbędne.

To może wyglądać na czepiactwo, ale w dużych projektach te wszystkie niekonsekwencje i swoboda nazewnictwa nawarstwiają się, pomnożone dodatkowo przez liczbę osób, i powoli zjadają programistów.

Zaznaczam, że nie jestem Javowcem webowcem - jeśli coś z tego, co wytknąłem, jest akurat kwestią konwencji frameworka, to sorry.

0

@biernacikkk: widzę że troche nie znasz architektury Springa/JEE
W warstwie serwisów mamy transakcje i jest ona po to żeby odpowiadać za logikę biznesową, i co prawda nie raz się zdarza że wywołuje tylko metodę z Repository, ale np. służy do tego że gdy masz to zamawianie towarów możesz w jednej metodzie zmniejszyć liczbę dostępnych produków w sklepie, zmienić stan koszyka klienta i wysłać emaila z potwierdzeniem chęci zakupu.
Trzeba też sprawdzić dostępność produktów żeby klient nie mógł zamówić więcej niż może. I zalecam albo zwracanie odłączanych encji od kontekstu JPA (detached) albo zwracanie DTO z serwisów :)

1

Nie ma testów, chociaż z drugiej strony za bardzo nie ma czego testować ;)
Sformatuj kod
Nazwanie klasy *Service i wrzucenie tam całej logiki nie ma najmniejszego sensu. Nie wiadomo wtedy co taka klasa robi, poza tym że może robić wszystko. U ciebie jest to po prostu przelotka do repozytorium, pytanie czy kontroler nie mógłby mieć od razu zależności do repozytorium.

Co dalej? Rób to co chcesz - logowanie i koszyk to dobry pomysł. Poczytaj o testowaniu kodu np. Practical unit testing with JUnit and Mockito oraz Test Driven Development by example i przetestuj swój kod. Clean code to też pozycja, którą mogę polecić jeżeli jeszcze nie czytałeś. A poznawanie Springa/innych FW i bibliotek zostaw na później ;) Jak ci brakuje pomysłów to możesz zrobić np. zwrot produktu, powiadomienie o ostatnich sztukach czy zrobić sobie integracje z jakąś bramką płatności chociażby sms.

0

No nie zgadzam się z tym do końca odnośnie serwisów. Zawsze można podzielić na kilka klas jeśli czegoś jest dużo, albo zmienić nazwy klas, service to jest to prostu taki obiekt to logiki. Nikt nie każee robić OrderService może być np. OrderProcessingService i wiesz że logika związana z tym że ktoś zamawia produkt i musisz sprawdzić stan na magazynie, zmienić ilośc dostępnych produktów itd. to musisz skorzystac z tej klasy :)

0

@scibi92:
Jeżeli ktoś zrobi OrderProcessingService i np. OrderSupplierService zamiast wrzucić wszystko do jednej klasy i nazwać to OrderService - to dla mnie nie ma problemu. Co prawda nazwalbym to po prostu OrderProcessor, ale to detal, mi chodziło o robienie generycznych god objectów typu OrderService, ProductService bez jakiegokolwiek podziału odpowiedzialności.

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