Conversation
| new FilmGenreRowMapper()); | ||
|
|
||
| // пополням фильмы сведениями о жанрах | ||
| for (FilmGenre filmGenre : filmsGenres) { |
There was a problem hiding this comment.
(Предложение)Данный функционал дублируется в коде. К тому же жанры не совсем подходят под данный репозиторий. Можно вынести метод по получению жанров в GenreStorage и обогащать фильмы уже в сервисе. Но очень хорошо сделано, что обращение к бд идет не в цикле, а за константное количество запросов =)
There was a problem hiding this comment.
Повторяющийся код оптимизировал. Зачем нужен отдельный репозиторий для таблицы связи мне не понятно.
There was a problem hiding this comment.
Я не совсем про новый репозиторий. В проекте уже есть GenreDbStorage, и было бы логичнее получение жанров вынести в данный репозиторий.
| */ | ||
| @Slf4j | ||
| @Service | ||
| public class FilmService { |
There was a problem hiding this comment.
Для сервисов также было бы хорошо использовать интерфейсы
| private final UserStorage users; | ||
|
|
||
| public FilmService(FilmStorage filmStorage, UserStorage users) { | ||
| public FilmService(@Qualifier("filmDbStorage") FilmStorage filmStorage, |
There was a problem hiding this comment.
Все InMemory реализации необходимо убрать из проекта, после этого @qualifier будет не особо нужен
There was a problem hiding this comment.
Удалил реализации InMemory.
| @@ -72,7 +76,6 @@ public Film updateFilm(Film updFilm) { | |||
| Film film = films.getFilmById(id).orElseThrow(() -> | |||
There was a problem hiding this comment.
.orElseThrow(() лучше начинать с новой строки
| @@ -104,8 +116,7 @@ public Integer addNewLike(Integer filmId, Integer userId) { | |||
| users.getUserById(userId).orElseThrow(() -> | |||
There was a problem hiding this comment.
На 114 строчке переменная film нигде не используется в дальнейшем
|
|
||
| @Component | ||
| @Repository("inMemoryFilmStorage") | ||
| public class InMemoryFilmStorage implements FilmStorage { |
There was a problem hiding this comment.
Все InMemory реализации необходимо удалить
There was a problem hiding this comment.
Удалил реализации InMemory.
| import java.util.*; | ||
|
|
||
| @Component | ||
| @Repository("inMemoryUserStorage") |
There was a problem hiding this comment.
Все InMemory реализации необходимо удалить
There was a problem hiding this comment.
Удалил реализации InMemory.
src/main/resources/schema.sql
Outdated
| -- Создаем таблицу пользователей | ||
| CREATE TABLE IF NOT EXISTS users ( | ||
| id INTEGER GENERATED ALWAYS AS IDENTITY PRIMARY KEY, | ||
| email VARCHAR(255) NOT NULL, |
There was a problem hiding this comment.
Исправил
....
email VARCHAR(255) UNIQUE NOT NULL,
....
| return films.addNewLike(filmId, userId); | ||
| } | ||
|
|
||
| public Integer removeLike(Integer filmId, Integer userId) { |
There was a problem hiding this comment.
переменная film нигде не используется в методе, можно ее не заводить
There was a problem hiding this comment.
(Предложение)Упустил пару моментов в предыдущем ревью. Можно валидировать не только объекты, которые приходят в теле запроса, но и параметры, такие как count в методе findPopularFilms
There was a problem hiding this comment.
Надеюсь я правильно понял.
http://localhost:8080/films/popular?count=-1
{
"error": "400 BAD_REQUEST "Validation failure""
}
2 participants
Спринт 12. часть 1. добавление базы данных.