#programowanie #java #spring #codereview Kiedyś już prosiłem o cod... (@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 :)

Najlepsze
Wszystkie komentarze

selenita66

selenita66 8 l.

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

mediateh7

mediateh7 8 l.

@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

Roballo

Roballo 8 l.

@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

selenita66 8 l.

@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

Roballo 8 l.

@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

k.....e

konto usunięte 8 l.

@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

Roballo

Roballo 8 l.

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

selenita66

selenita66 8 l.

@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ć. ;)

Seveir

Seveir Autor 8 l.

@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