search

Cha0z / springBootHomeWork

0 stars 1 forks source link

Code review #1

Open hjvfyfyfy opened 5 years ago

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/resources/application.properties#L2

Має бути datasourсe

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/models/ClothesSex.java#L3 ClothesSex має бути в окремому пакеті + зазвичай пишуть ClothesSexEnum щоб явно дати зрозуміти що то Enum

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/models/Water.java#L23

Float volume; зазвичай використовують Double, оскільки ти використовуєш не примітиви економія памяті виходить мізерна

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/models/Pants.java#L13

Назви в моделях зазвичай в однині, крім деяких рідких випадків. Аналогічно і назви класів, крім enums. Знаєш чому?

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/service/ShopService.java#L10

при сейві завжди вертай збережений обєкт Shop save(Shop shop); інколи буває тре отримати id нового обєкта

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/service/ShopServiceImpl.java#L12

Гарна практика засовувати такі класи в саб-пакет (impl)

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/web/controller/ShopController.java#L1

чому package com.homework.home.web.controller а не package com.homework.home.controller ?

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/web/controller/ShopController.java#L11

тут треба додати @RequestMapping("/shop/") таким чином ти розмежовуєш відповідальність між контролеерами

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/web/controller/ShopController.java#L19-L22

завжди вертай щось в таких методах, треба завжти валідувати реквести

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/web/controller/ShopController.java#L31-L35

коли ти пишеш @GetMapping це вже значить що ти чочеш отримати обєкт. Мало б бути щось типу @GetMapping("/{name}"). Анкалогічно @PutMapping("/add") і @DeleteMapping("/delete")

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/web/controller/ShopController.java#L31-L35

Але тут є і інша проблема в тебе респонсе List. Коли пишуть @GetMapping("/{name}") мається на увазі що "name" це id і повернеться тільки один обєкт. А в тебе виходить що то параметрер пошуку і це сильно руйнує логіку. Наприклад часто можна побачити url щось типу /shop/testshop/department/A --- тобто тобі треба вернути відділ A в магазині testshop, а як в твому випадку виглядавби цей запит ?

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/web/controller/ShopController.java#L37-L41

в тебе посуті name не id і manufacturer не id, але один параметре ти передааєш через RequestParam а інший через PathVariable чому ?

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/aa67418b3f06ca20557397d165b4a4b56e158588/src/main/java/com/homework/home/web/controller/ShopController.java#L25-L27

не пиши так, ти апсолютно не контролюєш процес видалення обєкти, і навіщо тобі писати @RequestBody Shop shop якщо тобі потрібно тільки id для видалення.

hjvfyfyfy commented 5 years ago

######################## MAP ######################## https://github.com/Cha0z/springBootHomeWork/blob/4c9d4c7305a48d4dff477024e98f6d4a8b501951/src/main/java/com/homework/home/map/OwnMap.java#L6

я трохи інше мав на увазі, я хотів щоб ти реалізував HashMap для того щоб ти розібрався і зрозумів як вона пряцю в всіх деталях, а погано пояснив що то має бути, якщо мажеш перероби, реалізуй методи put, get, remove, containsKey, size, values. ти розібрався з іншими колекціями і з їх швидкодією?

hjvfyfyfy commented 5 years ago

https://github.com/Cha0z/springBootHomeWork/blob/4c9d4c7305a48d4dff477024e98f6d4a8b501951/src/main/java/com/homework/home/map/OwnMap.java#L19

стандартний метод put маєж іншу сігнатуру

hjvfyfyfy commented 5 years ago

на майбутнє : добавити дженерики