Pierwszy projekt w Springu, proste api, crud

0

Cześć. Napisałem swój pierwszy projekt w Springu. Jest to proste crudowe api (to do list). Mógłby ktoś zrobić mi code review i powiedzieć czy jest w miarę ok a jak nie to co poprawić?
Link do Githuba: https://github.com/Edelner/To-do-list-web-spring-boot-app

2

Ale co ty tam chcesz poprawiać? Przecież tam nie ma żadnego kodu. Nie ma co sprawdzać. Mogłeś dać tam w ogóle Spring-Data-Rest zamiast samego Spring-Data i w ogóle w całym projekcie byłyby 3 pliki xD
Jedyne co można skomentować to wrzucenie target do gita...

3

Jest w miarę ok.

Najistotniejsze:

  • Następnym razem daj sensowne commit message
  • Wywal target
  • TaskCrudService metoda readById bez sensu, że zwracasz nowo utworzone zadanie gdy nie znajdzie po id, zmieniłabym to, żeby metoda zwracała optionala, byłoby to bardziej logiczne/intuicyjne.
  • TaskCrudService metoda updateDescription - zamiast usuwać i tworzyć nowe zadanie, powinieneś tylko zrobić update, wystarczy zrobić samo save.

Takie detale:

  • TaskToDo to słaba nazwa dla encji, ja uprościłabym do Task, bo jak w końcu ktoś tego taska zrobi, to nie będzie todo (przypuszczam, że jak będziesz rozwijał projekt to dodasz pole "status").
  • od springa 4 kropka coś tam, nie musisz dodawać adnotacji @Autowired jak wstrzykujesz przez konstruktor i masz jeden konstruktor
  • Ja osobiście nie tworzę interfejsu jeżeli mam tylko jedną implementację (to w kontekście TaskCrud)
3

Albo to ten sam bootcamp dał to samo zadanie albo Dzień Świstaka
Projekt OPa ma <24 godzin, a kilka dni temu był prawie identyczny projekt i temat i to samo do skomentowania

Niżej CAŁY kod

class MyProjectsToDoListApplicationTests {

    @Test
    void contextLoads() {
    }

}
5

0

Po co ci interfejs TaskCrud?

2
hzmzp napisał(a):

Po co ci interfejs TaskCrud?

Bo tak niestety w tutkach wszędzie robią tylko jest zazwyczaj konwencja na zasadzie TaskService i TaskServiceImpl ;p Sam jak 2 lata temu zaczynałem przygodę z programowaniem to myślałem, że tak się po prostu robi :D

3

Moim zdaniem w ten sposób tworzysz masę śmieciowego kodu. Nie szybciej i czyściej jest zrobić klasę, a dopiero w momencie potrzeby podmiany implementacji zrobić z tego ~Impl i interfejs wygenerować?

1
Skoq napisał(a):

Sam jak 2 lata temu zaczynałem przygodę z programowaniem to myślałem, że tak się po prostu robi :D

No i takie podejście ma nawet nazwę: cult cargo. Robimy coś, bo tak było robione wcześniej, ale nie ma to żadnej konkretnej przyczyny.

Edelner napisał

Moim zdaniem chyba lepiej, żeby był intefejs i mieć tą dodatkową warstwę abstrakcji, żeby w razie czego podmienić tę implementację.

Na czym polega ta warstwę abstrakcji, skoro ten interfejs leży dokładnie obok jedynej klasy, która go implementuje? Co od czego abstrahujesz? Jak wymienisz implementację, skoro jakakolwiek nowa implementacja będzie musiała mieć zależność od starej (bo tam jest interfejs)?

0

@szarotka: Przecież jak chcę zaktualizować opis zadania i zachować przy tym niemutowalność, to najpierw muszę usunąć stare zadanie ze starym opisem i potem zapisać nowe z nowym opisem. Jakbyś to inaczej zrobiła?

@somekind Zobacz. Na razie serwis korzysta z repozytorium Spring Data i zapisuje zadania do bazy danych. Jeśli ktoś chciałby mieć repozytorium w pamięci w postaci np. listy, to tworzy sobie nową klasę np. TaskCrudInMemoryService i implementuje interfejs TaskCrud. Poprzedni serwis zostaje nienaruszony. Klasa controllera również, bo ma wstrzyknięty intefejs i nie obchodzi ją, gdzie ani jak zapisywane są taski.

5
Edelner napisał(a):

@szarotka: Przecież jak chcę zaktualizować opis zadania i zachować przy tym niemutowalność, to najpierw muszę usunąć stare zadanie ze starym opisem i potem zapisać nowe z nowym opisem. Jakbyś to inaczej zrobiła?

Niemutowalność obiektu w javie nie implikuje że ten obiekt nie może zmienić stanu w bazie. Tworzysz nowy obiekt w javie ale nie w bazie danych.

1
Edelner napisał(a):

@somekind Zobacz. Na razie serwis korzysta z repozytorium Spring Data i zapisuje zadania do bazy danych. Jeśli ktoś chciałby mieć repozytorium w pamięci w postaci np. listy, to tworzy sobie nową klasę np. TaskCrudInMemoryService i implementuje interfejs TaskCrud. Poprzedni serwis zostaje nienaruszony. Klasa controllera również, bo ma wstrzyknięty intefejs i nie obchodzi ją, gdzie ani jak zapisywane są taski.

Ale tu pomieszałeś, za przechowanie odpowiada warstwa pod repozytorium a nie serwis!

3
Edelner napisał(a):

@somekind Zobacz. Na razie serwis korzysta z repozytorium Spring Data i zapisuje zadania do bazy danych. Jeśli ktoś chciałby mieć repozytorium w pamięci w postaci np. listy, to tworzy sobie nową klasę np. TaskCrudInMemoryService i implementuje interfejs TaskCrud. Poprzedni serwis zostaje nienaruszony. Klasa controllera również, bo ma wstrzyknięty intefejs i nie obchodzi ją, gdzie ani jak zapisywane są taski.

To jak będziesz to potrzebować to wtedy dodaj interfejs i druga implantację, a póki co - YAGNI.

0
Edelner napisał(a):

@somekind Zobacz. Na razie serwis korzysta z repozytorium Spring Data i zapisuje zadania do bazy danych. Jeśli ktoś chciałby mieć repozytorium w pamięci w postaci np. listy, to tworzy sobie nową klasę np. TaskCrudInMemoryService i implementuje interfejs TaskCrud. Poprzedni serwis zostaje nienaruszony. Klasa controllera również, bo ma wstrzyknięty intefejs i nie obchodzi ją, gdzie ani jak zapisywane są taski.

Ale dlaczego?
Dlaczego powodem zmiany serwisu ma być zmiana repozytorium? Co serwis obchodzi, gdzie i jak są trzymane dane?
To nie jest abstrakcja, to jest antyabstrakcja.

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