Add friends likes #1
Conversation
| @Service | ||
| public class UserService { | ||
|
|
||
| protected static UserStorage users = Storages.getUerStorage(); |
There was a problem hiding this comment.
Не стоит вручную создавать объекты, Spring сам умеет внедрять необходимые зависимости
| */ | ||
| public User updateUser(User updUser) { | ||
| Integer id = updUser.getId(); | ||
| User user = new User(users.getUserById(id)); |
There was a problem hiding this comment.
Не обязательно при обновлении делать копию объекта, можно работать с текущим. В будущем, когда будет работа с бд и транзакциями, будет гораздо удобнее работать с объектом полученным из бд
There was a problem hiding this comment.
Убрал копирование при обновлении. Получается, что исправляем сразу объект в хранилище (даже put(...) не нужен).
| public class FilmController { | ||
|
|
||
| @Autowired | ||
| FilmService service; |
| public class UserController { | ||
|
|
||
| @Autowired | ||
| UserService service; |
There was a problem hiding this comment.
Также можно сделать private final
|
|
||
| public Collection<Film> findPopularFilms(int count) { | ||
| List<Film> popularFilms = new ArrayList<>(); | ||
| popularFilms = films.findAllFilms().stream() |
There was a problem hiding this comment.
Не самы лучший подход. Например, если пользователь запросит топ 10 фильмов, то из хранилища будут извлечены все имеющиеся. Лучше фильтровать на уровне хранилища, от туда должны приходить только необходимые данные
There was a problem hiding this comment.
Реализовал обновляемый рейтинг в хранилище и выдачу заданного числа самых популярных фильмов.
| */ | ||
| public T addNew(T element) throws ValidationException { | ||
| // Проверяем существование полльзователя для исключения дублирования | ||
| if (storage.containsValue(element)) { |
There was a problem hiding this comment.
(Предложение)По идее, проверка должна происходить на уровне сервиса через Storage отдельным методом. Методы в хранилище должны быть простыми и работать с уже готовыми данными
There was a problem hiding this comment.
Перенес проверки в сервис.
|
|
||
| Integer id = element.getId(); | ||
| // проверяем необходимые условия | ||
| if (!storage.containsKey(id)) { |
There was a problem hiding this comment.
(Предложение) Аналогично, было бы логичнее проверку на наличие вынести в сервис
There was a problem hiding this comment.
Перенес проверки в сервис.
| /** | ||
| * Утилитарный класс для определения реализаций хранилищ данных | ||
| */ | ||
| public final class Storages { |
There was a problem hiding this comment.
Фабрика в данном проекте особа не нужна, Spring сам управляет бинами
There was a problem hiding this comment.
Удалил классы: Storages.java, StorageData, InMemoryAbstractStorage.java
There was a problem hiding this comment.
StorageData.java пришлось восстановить, так как в противном случае Lombok менял число аргументов в стандартных конструкторах классов модели ..
Спринт №11 итоговый проект.