[Seveir]

#programowanie #java #spring #codereview

Kiedyś już prosiłem o code review mojego innego projektu i pamiętam, że dało mi to dużo wskazówek do przemyślenia. Dlatego zwracam się do Was lepsi programiści o zerknięcie na mój projekt API do zarządzania wydatkami: .

Tutaj sobie to implementuje używając Angulara, więc można się pobawić.

Takie główne pytania:
1. Czy dobrze podzieliłem kod pomiędzy ExpenseService i ExpenseController?
2. Chciałbym dodać użytkowników, tak żeby każdy miał tylko i wyłącznie do swojej bazy wydatków. Stąd pytanie czy dobrze będzie jak do wydatków będzie kierował link users/{user-id}/expenses?
3. Potrzebuję w ogóle podpowiedzi co do Security. Jakim sposobem zabezpieczyć takie pojedyncze bazy danych, jak uwierzytelniać logowanie itp.

Z góry dziękuję za pomoc :)

[selenita66]

@Seveir: Najpierw to zrób sobie language review i pozmieniaj te bolesne, wszechobecne ortografy... "expenses", nie "expences".

[mediateh7]

@Seveir: edytowanie wydatkow czy parsowanie dat nie powinno sie odbywac w kontrolerach tylko serwisach, w kontroler nie chcesz mieć logiki.
no i tak selenita mowi to Expence niemilosiernie razi w oczy

[Seveir]

@selenita66: słuszna uwaga. Dziś się właśnie za to zabrałem, bo mnie to już męczyło, ale nie znalazło się to jeszcze na masterze. https://github.com/Seveir/expences-API/tree/add/login

[Roballo]

@Seveir:
tak na szybko:
-brak walidacji po angularowej stronie
- ux leży (warto poczytać ewentualnie popatrzeć na konkurencyjne strony),
-wywalasz stack co ci się dokładnie wysypało do klienta ->
https://expences-app.herokuapp.com/add-expenses dodaj + add all zwraca 500 + stack co

[selenita66]

@Seveir: A tak bardziej technicznie (bez wgryzania się w kod), odnośnie drugiego pytania. To, co proponujesz, a więc endpoint w postaci users/{user-id}/expenses, sugeruje że user jest zasobem z punktu widzenia samego użytkownika Twojego API. Myślę, że to zbędne i znacznie trafniejsze byłoby wycięcie users/{user-id} i zostawienie samego expenses. Natomiast to, co byłoby zwracane, powinno być filtrowane zależnie od tego kto jest aktualnie zalogowany.

[Roballo]

@GetMapping("/category/{id}/days/{dayFrom}/{dayTo}")
-poczytaj o dobrych praktykach restowych, w tym wypadku daty jako requestParam
-korzystaj z optionali
- patrzyles w baze ile hibernate ci tabel

[Seveir]

@Roballo: @selenita66: Dzięki, już mam jakieś kolejne kroki do podjęcia. Mogę Was zawołać za jakiś czas, jak pchnę się trochę do przodu?

[Roballo]

@Seveir: pod warunkiem, że drugi raz nie popełnisz tych samych błędów ( ͡° ͜ʖ ͡°)

[selenita66]

@Seveir: To raczej @Roballo wołaj, ja jestem świeżakiem w kwestii RESTowych API, niedawno stawiałem dopiero pierwsze kroki. A Javy nie znam. Za to uwielbiam się czepiać. ;)

[Godziu73]

@Seveir: nowatorski ten crud xD

[Seveir]

@Godziu73: na czymś trzeba ćwiczyć ¯\_(ツ)_/¯

[Godziu73]

@Seveir: a potem przychodzą na rozmowę tacy ludzie co sie nadają do generowania geterow co najwyżej

[k.....e]

@Seveir:

* Po co używasz optionali jak i tak walisz od razu get bez sprawdzania czy coś jest? ¯\_(ツ)_/¯
* Nie widzę żeby była jakakolwiek obsługa wyjątków
* Mieszasz modele, z encjami i DTO
* Nie używasz operacji kaskadowych więc jak wywalisz kategorię to wszystkie wydatki pewnie nadal zostaną w bazie chociaż nie będzie

[Seveir]

@kebab-case: o jaa, dziękuję. Będzie dużo pracy :)