Develop by Ilusha92 · Pull Request #15 · Ilusha92/java-filmorate

Ilusha92 · 2023-02-22T14:02:24Z

No description provided.

add base methods - no implement

implemented getRecommendedFilms method - pass 364 postman tests

without validation for existence of users and friendship

simplified bd request sorting List by likesCount

feat: добавлена возможность удаления фильмов и пользователей

* Добавлен класс director и DirectorDbStorage * Fixed not found status --------- Co-authored-by: Aleksandra <rok@bk,ru> Co-authored-by: kasisaki <32216047+kasisaki@users.noreply.github.com>

# Conflicts: # src/main/java/ru/yandex/practicum/filmorate/dao/FilmDbStorage.java # src/main/java/ru/yandex/practicum/filmorate/model/Film.java # src/main/java/ru/yandex/practicum/filmorate/service/FilmServiceManager.java # src/main/java/ru/yandex/practicum/filmorate/service/UserService.java # src/main/java/ru/yandex/practicum/filmorate/storage/FilmStorage.java # src/main/java/ru/yandex/practicum/filmorate/storage/InMemoryFilmStorage.java # src/main/resources/application.properties # src/main/resources/schema.sql

Add reviews!

add user existence check add getting review user id before update

replaced some constructors with lombok annotation simplified checkUserInDb method removed commented code some other small changes

Add feed

final ver

all tests ok

add search by title and director

all develop tests ok

restructed schema

avfyodorov

Добрый вечер, Илья!

Перво-наперво хочу извиниться за задержку. Честно говоря, я не знаю как ваш проект попал ко мне, ведь я не ваш ревьюер. Видимо, праздники влияют. 🙂 Ладно, это пусть начальство разбирается. Чтобы вас не задерживать, я провёл ревью, а в дальнейшем, надеюсь, ваш проект вернётся к штатному ревьюверу.

Теперь по самому проекту. Видно, что вы проделали большую работу, реализовали функциональность, прописанную в Техническом задании, а, с учётом того, что команда у вас несыгранная, и это первый ваш групповой проект, результат можно считать хорошим.

Тем не менее, есть несколько вопросов.

Репозитарии должны работать с таблицами базы данных, каждый репозитарий со своей таблицей, и они все друг от друга не зависят. Постарался описать это в своих комментариях в коде.
Всю бизнес логику нужно сосредоточить в слое Сервисов. Это их зона ответственности.
Большая просьба описать в readme реализованную функциональность, хотя бы кратко. Кто и что сделал. Я не нашёл.
не смог сходу разобраться зачем у вас в проекте два комплекта репозитариев: один для работы с базой данных, а другой, Судя по всему, остался с первого спринта. Было бы хорошо провести рефакторинг и устранить лишнее.
Крайне желательно избавляться от большого количества запросов к серверу в цикле. Конечно, не нужно тащить всю базу данных к себе в память, это тоже не дело, но, общая идея в том, чтобы сократить количество запросов до минимально разумного.

Не стал писать одинаковые комментарии в разных классах.Поэтому прошу Вас проверить аналогичные ситуации в других частях проекта.

Вот вкратце, что я нашёл. Конечно, есть что поправить, но в целом всё очень неплохо. Поздравляю с удачным опытом групповой разработки, успехов!

src/main/java/ru/yandex/practicum/filmorate/controller/DirectorController.java

src/main/java/ru/yandex/practicum/filmorate/controller/FilmController.java

src/main/java/ru/yandex/practicum/filmorate/controller/DirectorController.java

src/main/java/ru/yandex/practicum/filmorate/controller/FilmController.java

src/main/java/ru/yandex/practicum/filmorate/dao/DirectorDbStorage.java

src/main/java/ru/yandex/practicum/filmorate/dao/UserDbStorage.java

avfyodorov · 2023-02-23T18:27:02Z

src/main/java/ru/yandex/practicum/filmorate/dao/mappers/FilmMapper.java

            film.setId(rs.getInt("filmId"));
            film.setMpa(filmStorage.getMpaById(rs.getInt("mpaId")));
-            SqlRowSet getFilmGenres = jdbcTemplate.queryForRowSet("SELECT genreId FROM film_genre WHERE filmId=?", film.getId());
+            SqlRowSet getFilmGenres = jdbcTemplate.queryForRowSet("SELECT genreId FROM film_genre WHERE filmId = ?", film.getId());


Вот здесь в цикле на каждую запись набора дёргается сервер с отдельным запросом. Это не очень хорошо. Лучше было бы придумать такой запрос, чтобы получить нужные данные сразу. И уже в памяти, если нужно, отфильтровать их.

avfyodorov · 2023-02-23T18:31:04Z

src/main/java/ru/yandex/practicum/filmorate/dao/mappers/FilmMapper.java

-            SqlRowSet getFilmGenres = jdbcTemplate.queryForRowSet("SELECT genreId FROM film_genre WHERE filmId=?", film.getId());
+            SqlRowSet getFilmGenres = jdbcTemplate.queryForRowSet("SELECT genreId FROM film_genre WHERE filmId = ?", film.getId());
            while(getFilmGenres.next()){
                Genre genre = filmStorage.getGenreById(getFilmGenres.getInt("genreId"));


А здесь ещё и вложенный цикл и внутри него обращение к репозитарию...

avfyodorov · 2023-02-23T18:34:20Z

src/main/java/ru/yandex/practicum/filmorate/dao/review/ReviewDbStorage.java

+    private static long REVIEW_ID;
+    private final NamedParameterJdbcTemplate namedParameterJdbcTemplate;
+    private final JdbcTemplate jdbcTemplate;
+    private final ReviewLikesStorage reviewLikesStorage;


Повторюсь: в репозитории только обращение к собственной таблице, никаких зависимостей от других репозитариев быть не должно. Если нужно собирать данные из нескольких репозитариев и это нельзя сделать одним запросом, то скорее всего это зона ответственности соответствующего сервиса.
Вся бизнес логика должна находиться в слое сервисов.

src/main/resources/schema.sql

changelog in telegram for internal use only

# Conflicts: # src/main/resources/schema.sql

добавлены тесты

# Conflicts: # src/main/java/ru/yandex/practicum/filmorate/controller/DirectorController.java # src/main/java/ru/yandex/practicum/filmorate/controller/FilmController.java # src/main/java/ru/yandex/practicum/filmorate/controller/GenreController.java # src/main/java/ru/yandex/practicum/filmorate/controller/MpaController.java # src/main/java/ru/yandex/practicum/filmorate/dao/FilmDbStorage.java # src/main/java/ru/yandex/practicum/filmorate/dao/UserDbStorage.java # src/main/java/ru/yandex/practicum/filmorate/dao/review/ReviewDbStorage.java

refactor dependencies, add logging, join kasisaki-igorshmidt99-alex9379992 changes

changed dependencies removed loop access to DB

refactor develop

avfyodorov

Ещё раз добрый день, Илья!

Работу принимаю.
Поздравляю Вас и вашу группу с окончанием проекта.

avfyodorov · 2023-02-25T08:43:54Z

src/main/java/ru/yandex/practicum/filmorate/controller/FilmController.java

    private final FilmService filmService;

+    @GetMapping("/common")
+    public List<Film> getCommonFilms(@RequestParam("userId") int userId, @RequestParam("friendId") int friendId) {


Просил добавить логирование, ладно….

avfyodorov · 2023-02-25T08:54:01Z

src/main/java/ru/yandex/practicum/filmorate/dao/DirectorDbStorage.java

+@Slf4j
+public class DirectorDbStorage implements DirectorStorage {
+    private final JdbcTemplate jdbcTemplate;
+    private final FilmMapper filmMapper;


Ну да, конечно, filmMapper…..
А filmMapper В свою очередь дёргает MpaService… Молодцы, спрятались 🙂

Чем программа стройнее, прямее, без хитрых зависимостей, тем она надёжнее и проще в сопровождении.

avfyodorov · 2023-02-25T08:55:49Z

src/main/java/ru/yandex/practicum/filmorate/dao/DirectorDbStorage.java

+    @Override
+    public List<Director> findAll() {
+        String statement = "SELECT * FROM directors";
+        List<Director> directors = jdbcTemplate.query(statement, new DirectorMapper());


DirectorMapper
Там же всего 1 метод... Можно было его прямо сюда и включить. Тем более, что по смыслу он как раз здесь и к месту .

alex9379992 and others added 23 commits February 15, 2023 21:47

feat: добавлена возможность удаления фильмов и пользователей

cc340b4

add-recommendations

18a27c4

add base methods - no implement

add-recommendations

84a304b

implemented getRecommendedFilms method - pass 364 postman tests

get-add-common-films

ba12d81

without validation for existence of users and friendship

get-add-common-films

4ffb8a3

without validation for existence of users and friendship

get-add-common-films

a3dbd23

without validation for existence of users and friendship

get-add-common-films

f8f02db

simplified bd request sorting List by likesCount

Merge pull request #7 from Ilusha92/add-remove-endpoint

4963073

feat: добавлена возможность удаления фильмов и пользователей

feat: реализовать ТЗ

d559d60

feat & fix: исправить согласно тестам

aed5f21

Add director (#9)

760e987

* Добавлен класс director и DirectorDbStorage * Fixed not found status --------- Co-authored-by: Aleksandra <rok@bk,ru> Co-authored-by: kasisaki <32216047+kasisaki@users.noreply.github.com>

fix: исправить замечания по контроллеру и сервису

ce692e9

Merge pull request #11 from Ilusha92/add-reviews

374aa4c

Add reviews!

fead: добавлены Events

83aefe0

add-feed

25f8e00

add user existence check add getting review user id before update

add-feed

534f5a3

replaced some constructors with lombok annotation simplified checkUserInDb method removed commented code some other small changes

Merge pull request #12 from Ilusha92/add-feed

c203e93

Add feed

add search by title and director

69fcb7b

final ver

декомпозирован код метода search

0fa59a0

all tests ok

Merge pull request #13 from Ilusha92/add-search

71f4046

add search by title and director

restructed schema

d609e78

all develop tests ok

Merge pull request #14 from Ilusha92/add-search

8a63d16

restructed schema

avfyodorov reviewed Feb 23, 2023

View reviewed changes

Igor Shmidt and others added 6 commits February 24, 2023 00:16

fix: перенести взаимодействия review & event dao в ReviewService

b41ed34

добавлены тесты

22af136

refactor develop

3cddda6

changelog in telegram for internal use only

fix: генерация ключей

7e20e67

refactor: удаления методов генерации ключей

b41ccdb

Merge branch 'develop-mutaev' into bug-fix-reviews

8492176

# Conflicts: # src/main/resources/schema.sql

Ilusha92 and others added 9 commits February 24, 2023 13:12

Update README.md

f3473d2

Merge pull request #16 from Ilusha92/develop-nazyan

f995940

добавлены тесты

refactor: устранить запросы внутри цикла

0ca95ff

Merge branch 'develop' into bug-fix-reviews

77d0c20

Merge pull request #20 from Ilusha92/develop-shmidt

a4c914d

refactor dependencies, add logging, join kasisaki-igorshmidt99-alex9379992 changes

Осталось разобраться с FilmMapper

60dbe89

refactor develop

46cdbc2

changed dependencies removed loop access to DB

Merge pull request #21 from Ilusha92/develop-refactor

a040e0e

refactor develop

avfyodorov reviewed Feb 25, 2023

View reviewed changes

Ilusha92 requested a review from avfyodorov May 13, 2024 19:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Develop#15

Develop#15
Ilusha92 wants to merge 38 commits intomainfrom
develop

Ilusha92 commented Feb 22, 2023

Uh oh!

avfyodorov left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

avfyodorov Feb 23, 2023

Uh oh!

avfyodorov Feb 23, 2023

Uh oh!

avfyodorov Feb 23, 2023

Uh oh!

Uh oh!

avfyodorov left a comment

Uh oh!

avfyodorov Feb 25, 2023

Uh oh!

avfyodorov Feb 25, 2023

Uh oh!

avfyodorov Feb 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

Ilusha92 commented Feb 22, 2023

Uh oh!

avfyodorov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

avfyodorov Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

avfyodorov Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

avfyodorov Feb 23, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

avfyodorov left a comment

Choose a reason for hiding this comment

Uh oh!

avfyodorov Feb 25, 2023

Choose a reason for hiding this comment

Uh oh!

avfyodorov Feb 25, 2023

Choose a reason for hiding this comment

Uh oh!

avfyodorov Feb 25, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

avfyodorov left a comment •

edited

Loading