Backend review

[sysmat]

• tudi server na root nivoju bi lahko imel README kratek opis kaj je uporabljeno, koko zagnati server
• demo ni najboljše ime
• glede mysql strežnika, bi lahko tudi docker, docker-compose uporabili za boljši DX
• java naming convention imena razredov, spremenljivk, ...
• REST API kovencija in springboot omogoča mapping na nivoju razreda, ta se v vsaki metedi ne ponavlja /api/... in glagoli kot co create, delete, ... se ne pojavljajo v url path-u

/api
  /vx - opcijsko določimo verzijo
      /room
         POST, GET, DELETE  roomId
         GET  /all
     /reservation
      POST, GET, DELETE  reservationId

• REST API všeč mi je da ste uporabili tudi različne HTTP statuse, ko gre kaj narobe
• manjkajo input validacije
• manjkajo testi
• Map.of in key kot stringi ni najboljša praksa, hitro se kaj spregleda, v take namene se raje naredi DTO s pomočjo record

[sysmat]

• @CrossOrigin(origins = "http://localhost:4200") fiksen host ni vredu, kako bi to rešili?

[kle-pra]

Všeč mi je

• da ste uporabili Spring Boot zadnjo verzijo
• uporaba JPA
• query, ki preverja overlap

Predlagani popravki

• class za model je konvecionalno v ednini, npr Reservation.java
• včasih je dobro t.i. bussiness logiko dati v svoj "service" layer, npr. ReservationService.java, kjer je vsa logika, saj je trenutno veliko kar v controller metodi createRoomTime (preverjanje, kreiranje). Tako je koda bolj obvladljiva kasneje.
• poimenovanje REST endpoint poti: priporočeno, da se ne uporablja glagolov create, delete itd, ampak kar metoda pove akcijo:
  • POST na /room ustvari
  • DELETE na /room izbriše
  • lahko se tudi uporabi kakšna konstanta za del poti "api/room", ki se ponavlja

[BrawnyMan]

Hvala za predloge. Jih bom v prihodnje upošteval.

• sprememba imena "demo" je nujna, strinjam se, da je trenutno ime ni primerno
• docker je odlična ideja na katero nisem pomislil, vendar se popolnoma strinjam s to idejo
• glede testov mi je zmanjkalo časa, vendar se jim bom v prihodnje posvetil več
• pri uporabi Map.of je bil ideja, da bi vrnil objekt s spremenljivko in vrednostjo. Vendar zdaj vidim, da bi bilo bolje vrniti dejanski objekt namesto uporabe Map.of.

Hvala za opozorilo. @CrossOrigin(origins = "http://localhost:4200") > @CrossOrigin(origins = "*")

[BrawnyMan]

Posodobil sem kodo.

Spremenil sem:

• ime Java projekta,
• spremenil imena poti,
• dodal modele (DTO),
• logika v serviceReservation.java,
• dodal teste,
• uporabil record (ne povsod, ker predstavljaj nekje dodatne probleme),

[kle-pra]

Hvala še za te popravke

• je sedaj boljše - controller je sedaj bolj pregleden, ko je logika v service layer, REST pathi so tudi lepši. Fajn da ste dodali DTO objecte - tu bi priporočal, da kjer je možno in je http status OK (default) se vrne kar DTO, brez da se ga ovije v ResponseEntity wrapper, je pa to bolj preferenca, ki malo poenostavi metodo (return type - npr. če bi jo testirali, je lažje tetsirati, če ni wrapped).
• Super, da service layer sedaj tetsirate, s tehniko mockanja.

Trenutno smo še v procesu in je kar nekaj kandidatov, vas pa bomo kmalu obvestili kako smo se odločili.