Добавлена функциональность сохранение состояния данных после перезапуска #5
Conversation
avfyodorov
left a comment
avfyodorov
left a comment
There was a problem hiding this comment.
Добрый день, Максим!
В целом хорошо, правда, есть ряд уточнений.
| public class FilmDbStorage implements FilmStorage { | ||
| private final JdbcTemplate jdbc; | ||
| private final MpaDbStorage mpaStorage; | ||
| private final GenreDbStorage genreStorage; |
There was a problem hiding this comment.
private final MpaDbStorage mpaStorage;
private final GenreDbStorage genreStorage;
Вообще говоря, нет, это нехорошо. Репозитории (DAO) ничего друг про друга знать не должны. Репозиторий выполняет crud команды для своей таблицы (набора данных) и, если надо, возвращает результаты в сервис.
Если же репозиторий вызывает другие репозитории, то есть фактически зависит от них, то это серьёзный повод пересмотреть архитектуру программы.
Вся бизнес логика находится в сервисах. Сервис может обращаться к другим сервисам, а также получать данные из нескольких репозиториев. Репозиторий же настроен на свою таблицу.
| List<Film> films = jdbc.query(sql, this::makeFilm); | ||
| for (Film f : films) { | ||
| f.setMpa(mpaStorage.findById(f.getMpa().getId())); | ||
| List<Genre> genres = genreStorage.getGenresForFilm(f.getId()); |
There was a problem hiding this comment.
В этом месте, в цикле, идут запросы к базе данных. Было бы хорошо избавиться от этих многочисленных запросов, которые сильно нагружают сервер. Хорошо бы переделать данный метод, чтобы уменьшить число обращений к БД .
| List<Film> films = jdbc.query(sql, this::makeFilm, count); | ||
| for (Film f : films) { | ||
| f.setMpa(mpaStorage.findById(f.getMpa().getId())); | ||
| List<Genre> genres = genreStorage.getGenresForFilm(f.getId()); |
There was a problem hiding this comment.
f.setMpa(mpaStorage.findById(f.getMpa().getId()));
List<Genre> genres = genreStorage.getGenresForFilm(f.getId());
Здесь та же история-запросы в цикле. Нужно постараться избавиться..Сократить количество запросов.
| if (film.getGenres() != null && !film.getGenres().isEmpty()) { | ||
| String sql = "INSERT INTO film_genres (film_id, genre_id) VALUES (?, ?)"; | ||
| for (Genre genre : film.getGenres()) { | ||
| jdbc.update(sql, film.getId(), genre.getId()); |
There was a problem hiding this comment.
Хорошо было бы сохранить данные одним запросом, например, с помощью конструкции:
jdbcTemplate.batchUpdate(............
| if (film.getGenres() != null) { | ||
| Set<Genre> genres = film.getGenres().stream() | ||
| .filter(Objects::nonNull) | ||
| .map(g -> genreStorage.findById(g.getId())) |
avfyodorov
left a comment
avfyodorov
left a comment
There was a problem hiding this comment.
Добрый день, Максим!
Замечаний нет.
Работа принята.
No description provided.