Conversation
EugeneLenkevich
left a comment
EugeneLenkevich
left a comment
There was a problem hiding this comment.
Привет, Вячеслав! Спасибо за PR.
Отличная работа! :)
Отметил несколько моментов, но на самом деле в первую очередь посмотри, пожалуйста, вот что
- исключение пусть у нас будет проверяемым
- аннотация @Valid
| @RequestMapping("/films") | ||
| @Slf4j | ||
| public class FilmController { | ||
| private final List<Film> films = new ArrayList<>(); |
There was a problem hiding this comment.
По списку же искать муторно - надо все элементы перебирать, лучше взять какую-нибудь из реализаций Map, HashMap, ну или если хочется выпендриться, то ConcurrentHashMap. Web - конкурентная среда, одновременно может приходить множество запросов, надо уметь работать и в многопоточной среде
|
|
||
|
|
||
| @PostMapping | ||
| public Film createFilm(@RequestBody Film film) { |
There was a problem hiding this comment.
А тут наверно надо добавить @Valid перед @RequestBody? :)
| if (film.getName() == null || film.getName().isEmpty()) { | ||
| throw new ValidationException("Название фильма не может быть пустым"); | ||
| } | ||
| if (film.getDescription().length() > 200) { |
There was a problem hiding this comment.
А если description == null, то мы получим ошибку :)
| @@ -0,0 +1,7 @@ | |||
| package ru.yandex.practicum.filmorate.exception; | |||
|
|
|||
| public class ValidationException extends RuntimeException { | |||
There was a problem hiding this comment.
А подходит ли для данной ситуации непроверяемое исключение?
Пользователь ввел некорректные данные - вполне себе штатная ситуация, пусть тут будет проверяемое исключение :)
extends Exception
Есть еще такой признак - если исключение есть в бизнес-процессе, то его стоит сделать проверяемым. Так мы на уровне компилятора говорим о возможных альтернативных ситуациях и обязываем вышестоящий обработать их или эскалировать еще выше. По большому счету это и не ошибки вовсе, а некоторые отклонения от основного пути пользователя.
Например, снимаем денежку с карты в банкомате
стандартный путь: приложили карту, получили денежку
возможные отклонения
- ввели сумму больше чем денег на балансе
- карта заблокирована
- ввели неверный пин-код
- и пр
пример ошибки: у банкомата пропало соединение с интернетом (может и так он будет работать, не в курсе, но пусть будет что не может), эту ситуацию обрабатывать бессмысленно кроме как отобразить пользователю сообщение что "атм не работает" и прекратить все текущие операции до устранения внешней причины. И в бизнес-процессе этой ситуации нет, это чисто техническая история. Вот это уже RuntimeException
| */ | ||
| @Getter | ||
| @Setter | ||
| @Data |
There was a problem hiding this comment.
@Data
С этой аннотацией надо поаккуратней. Она генерирует equals и hashCode на основании всех полей.
Однако, сущность - это не просто объект, это представление строки таблицы базы данных, а строку таблицы однозначно определяет ее идентификатор, и потому, соотвественно, для сущностей equals и hashCode стоит определять на основе id (первичного ключа).
Остальные поля, при этом, могут вообще быть разными, то есть могут существовать разные версии одного и того же объекта, но как это разруливатеся - это уже отдельная история.
Плюс еще некоторые поля могут быть ленивыми коллекциями и в момент обращения бдут генерироваться sql-запросы, которые и не нужны в данный момент
Т.е. лучше или переопределить руками или указать через аннотации как генерировать эти методы :)
EugeneLenkevich
left a comment
EugeneLenkevich
left a comment
There was a problem hiding this comment.
Теперь все отлично, спасибо :)
3 participants
No description provided.