Ocena małej architektury - spring boot, design pattern

0

Cześć.

Prośba o ocenę czy takie rozmieszczenie klas, zastosowanie wzorców jest ok. Osobiście mam sporo wątpliwości, szczególnie do takiej ilości implementacji List do takiej ilości kodu.
Głównie interesuje mnie ocena serwisu i jego powiązań.
Link do GH do projektu:
GitHub

1

Quality samego kodu nie jest złe. Nie jest może najlepsze ale widziałem gorsze. Może być natomiast lepsze.

  1. Wszystko wszędzie jest private. Proponuję obejrzeć film Jakuba jak to można znacznie lepiej zrobić:
    Tu jest najkrótsza wersja tego wykładu:

    Natomiast jak chcesz bardziej szczegółowo to poszukaj wystąpienie Kuby w Future Processing z Silesia JUG.

  2. Masz kilka takich metod:

@Scheduled(cron = "0 */2 * * * *")
    public void updateCollectionGame(){
       System.out.println("Update Collection");
        apiFootballFacade.updateGameCollection();
    }

Wyrzuciłbym wszelkiego rodzaju SOUTy stąd. Jeśli już chcesz mieć jakieś informacje to użyj logger.
Warto się jeszcze zastanowić kiedy coś powinno być logowane. Tutaj wyświetlasz komunikat że jakaś kolekcja jest aktualizowana a potem wykonujesz operację. Co jeśli aktualizacja lub jakakolwiek operacja po logu się nie powiedzie?

  1. DataParser korzysta z Date. Proponuję zacząć korzystać z klasy LocalDate/LocalTime/LocalDateTime z Javy 8.

  2. Url'e warto trzymać w jakimś propertisie żebyś nie musiał potem tego w kodzie zmieniać (ApiFootballUrls):

 urls.add("http://api.football-data.org/v2/competitions/" + K + "/matches?" +

Btw z czystego kodu pamietam że najpierw daje się metody publiczne a pod nimi metody które wywołują więc może drobiazg ale warto zamienić miejscami.

  1. W klasie ApiFootballPojo kolejność też do zmiany.

  2. Formatowanie w klasie ApiFootballFilter. Np. zamiast:

 public List<Game> getAllGames(List<Game> listOfGameEntity){
        return listOfGameEntity.stream().filter(game ->
                !game.getStatusMatch().equals("FINISHED")).collect(Collectors.toList());
    }

Rób:

 public List<Game> getAllGames(List<Game> listOfGameEntity){
        return listOfGameEntity.stream()
                               .filter(game -> !game.getStatusMatch().equals("FINISHED"))
                               .collect(Collectors.toList());
    }

Tym obserwatorom przyjrzę się trochę później.
Przy okazji mógłbyś wyjaśnić zdanie:

Osobiście mam sporo wątpliwości, szczególnie do takiej ilości implementacji List do takiej ilości kodu.

Bo kompletnie nie mam pojęcia o co chodzi.

0

Dziękuję ślicznie za odpowiedź :)

  1. Sporo się naoglądałem wystąpień z WJUG czy też z confitury, uważam to za świetne źródło informacji także, postaram się to wszystko poprawić zgodnie z tym nagraniem.
  2. Tak SOUTy wszystkie na pewno będą zastąpione logerem, to tylko tak na teraz, żeby widziec co sie dzieję na bieżąco przy Scheduled. BTW. Log4j czy jakaś inna bibliotek ?
  3. Ten DateParser w ogóle mi się nie podoba. Po prostu wiem, że jest tam dużo złych rzeczy. Na pewno do poprawy jest 'getDateFromJson' oraz 'getTimeFromJson'
    Wydaję mi się to obrzydliwe.
    Postaram się poprawić metody zgodnie z Twoimi podpowiedziami.
  4. OK. A czy w ogóle tworzenie linków w ten sposób i potem wysyłanie ich do RestTemplate jest poprawne ? Czy może istnieje bibliotek która będzie na pewno lepsza do tego ?
  5. OK
    6 .OK
  6. Np. w ApiFootballGameCollection na start inicjalizacja trzech list. Nie wiem, mam odczucie, że można to jakoś ładniej zrobić. 1.

To jest w ogóle gdzie 3 wersja tego małego programu, za każdym razem mam wewnętrzne uczucie, że coś jest strasznie brzydko. Przez co teraz zacząłem oglądać wystąpienia na temat DDD. Tylko mimo kilku nagrań, nie bardzo wiem jak to ugryźć.
Czy w ogóle jako potencjalny junior za jakiś czas, powinienm się zajmować takimi tematami, czy na moim poziomi i tak, zalety DDD i tak będą ciężkie do zrozumienia i lepiej zająć się czymś innym?

0

Czemu masz tylko 2 testy?
edit: do tego z tego co pobieznie widziałem anemiczne encje, czyli logika tylko w serwisach.
also unikaj nazw typu *Service - to absolutnie nic nie mówi czytającemu (oprócz tego, ze klasa robi zazwyczaj więcej niż powinna). Używaj nazw, które pomogą zrozumieć od razu po przeczytaniu nazwy do czego służy dana klasa, np. ApiFootballService ma zbyt wiele odpowiedzialnosci, fasada mogła by tak wyglądać.
Z kolei ApiFootballPojo spodziewałbym się w sumie jakiegoś DTO czy tam po prostu struktury danych, a jest klasa z logiką

0

W trakcie przerabiania Kaczanowskiego :) Będzie niedługo !

0
cayman25 napisał(a):

W trakcie przerabiania Kaczanowskiego :) Będzie niedługo !

EDIT:
Wydaję mi się, że poprawiłem wszystko oprócz sout. Souty na razie zostawiam mając w perspektywie dodanie sfl4j po napisaniu testów do programu albo jeszcze później :)
Dodatkowo podsyłam diagram powiązań.

ExternalApiService

screenshot-20181109134003.png

Dodanie URL do properties wymusiło na mnie dodanie @Component do dwóch klas. Pytanie czy można to zrobić w inny sposób bez @Value ?

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