Skip to content

Conversation

@alphadrow
Copy link
Owner

No description provided.

Copy link

@eugene-lenkevich eugene-lenkevich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Привет, Валерий , спасибо за PR

В целом все супер :)
Есть совсем небольшие замечания, посмотри комментарии ниже, пожалуйста

@@ -0,0 +1,7 @@
package ru.yandex.practicum.filmorate.exception;

public class FilmNotFoundException extends RuntimeException{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А необходимо ли тут (и ниже) непроверяемое исключение?
Пользователь ввел некорректные данные, фильм не найден и т.д. и т.п. - вполне себе штатная ситуация, пусть тут будет проверяемое исключение :)
extends Exception

}

public void like(long filmId, long userId){
if (filmStorage.getById(filmId).isPresent()){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут можно сделать несколько короче, раз уж Optional :)

ifPresent(...).orElseThrow()

https://ducmanhphan.github.io/2019-12-06-Best-practice-for-Optional-in-Java/#using-orElseThrow()-method

log.debug("film.getName().isEmpty(): {}", film.getName());
return false;
}
if (film.getName().isBlank()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут можно использовать
StringUtils.hasText
или же аналогичную функцию из apache commons
StringUtils.isNotBlank
=)

Copy link

@eugene-lenkevich eugene-lenkevich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Все супер, спасибо!

PS про исключения посмотри во это, может пригодиться :)
https://habr.com/ru/post/528116/

@alphadrow alphadrow merged commit 6fffe27 into main Jun 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants