-
Notifications
You must be signed in to change notification settings - Fork 0
Создал сервис для шеринга ShareIt. #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ViktorSalk
commented
Mar 24, 2025
- Создал ветку: add-controllers.
- Создал четыре пакета: item, booking, request и user.
- Создал DTO-объекты и мапперы для сущностей: ItemDto, UserDto, BookingDto, ItemRequestDto
- Создал тесты на новую функциональность.
1) Создал ветку: add-controllers. 2) Создал четыре пакета: item, booking, request и user. 3) Создал DTO-объекты и мапперы для сущностей: ItemDto, UserDto, BookingDto, ItemRequestDto 4) Создал тесты на новую функциональность.
|
|
||
|
|
||
| /** | ||
| * TODO Sprint add-controllers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В коммерческой разработке, когда код из TODO реализован, принято его удалять, чтобы он не путал других разработчиков.
Удали, пожалуйста, все TODO, которые были выполнены во всем проекте
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Убрал лишние TODO.
| @Override | ||
| public List<ItemDto> getAll(Long userId) { | ||
| List<ItemDto> items = new ArrayList<>(); | ||
| for (Item item : itemRepository.findAll()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В коммерческой разработке уже давно сложно встретить обычные циклы, так как в java 8 был представлен более мощный инструмент для работы с коллекциями - stream api. С помощью стримов и лямбда - выражений работать с коллекциями гораздо удобнее и нативно понятнее, поэтому важно уметь их применять в своем коде
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Использовал stream api в методе getAll().
| } | ||
|
|
||
| @Override | ||
| public ItemDto update(ItemDto itemDto, Long id, Long userId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Обновление полей лучше вынести в метод в маппере, так как он занимается перекладыванием полей из одного объекта в другой
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вынес метод Обновление полей в маппер, после внедрения MapStruct, генерируется автоматически.
| if (text.isBlank()) { | ||
| return searchedItems; | ||
| } | ||
| for (Item item : itemRepository.findAll()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Используй stream api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Улучшил код, используя Stream API.
| import ru.practicum.shareit.item.dto.ItemDto; | ||
| import ru.practicum.shareit.item.model.Item; | ||
|
|
||
| public class ItemMapper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Саморазвитие
В коммерческой разработке принято максимально избавляться от шаблонного кода, чтобы не перегружать и так большие и объемные проекты. Для маппинга сущностей часто используется библиотека Mapstruct, которая позволяет автоматически настраивать маппинг из одной сущности в другую с помощью описания интерфейсов и аннотаций.
Вот здесь есть отличный гайд для старта работы с библиотекой.
При наличии свободного времени можно поизучать и попробовать прикрутить в проект
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Спасибо)) Внедрил библиотеки MapStruct, теперь код маппинга генерируется автоматически и работает быстрее.
| } | ||
|
|
||
| @GetMapping | ||
| public List<ItemDto> getAll(@RequestHeader("X-Sharer-User-Id") Long userId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нужно вынести повторяющийся хэдер в отдельный класс и сделать его публичной константой, чтобы избежать дублирований в других классах. Чтобы быстро поправить все значения, можно воспользоваться горячими клавишами в IDEA: Ctrl+Shift+R
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вывел повторяющийся хэдер в отдельный класс.
| import java.util.Map; | ||
| import java.util.Optional; | ||
|
|
||
| public abstract class AbstractRepository<T, K> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здорово, что создал абстрактный класс 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Спасибо))
| @Override | ||
| public List<UserDto> getAll() { | ||
| List<UserDto> users = new ArrayList<>(); | ||
| for (User user : userRepository.findAll()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Используй stream api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Улучшил код, используя Stream API.
|
|
||
| @SpringBootTest | ||
| @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD) | ||
| class UserControllerTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Получились отличные тесты!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Спасибо))
|
|
||
| private void throwIfEmailNotUnique(User user) { | ||
| for (User userCheck : userRepository.findAll()) { | ||
| if (user.getEmail().equals(userCheck.getEmail())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Дополнительные оптимизации
Чтобы проверка почты проходила за O(1), нужно завести Set, куда при добавлении нового пользователя мы будем класть его email. Соответственно, перед тем как добавить пользователя, нужно будет посмотреть, есть ли уже в сете подобный email, если есть - то пользователь с таким email уже существует.
Так как поиск и вставка в сете работают в среднем за O(1), то наш алгоритм поиска одинаковых email тоже будет работать в среднем за O(1), а это гораздо быстрее, чем пройтись по всем элементам списка O(n)
В коммерческой разработке уже давно сложно встретить обычные циклы, так как в java 8 был представлен более мощный инструмент для работы с коллекциями - stream api. С помощью стримов и лямбда - выражений работать с коллекциями гораздо удобнее и нативно понятнее, поэтому важно уметь их применять в своем коде
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Оптимизировал проверку уникальности email с помощью Set для достижения O(1) сложности.
1) Убрал лишние TODO. 2) Вывел повторяющийся хэдер в отдельный класс. 3) Улучшил код, используя Stream API. 4) Вынес метод Обновление полей в маппер, после внедрения MapStruct, генерируется автоматически. 5) Использовал stream api в методе getAll(). 6) Оптимизировал проверку уникальности email с помощью Set для достижения O(1) сложности. 7) Внедрил библиотеки MapStruct, теперь код маппинга генерируется автоматически и работает быстрее.
1) Убрал лишние TODO. 2) Вывел повторяющийся хэдер в отдельный класс. 3) Улучшил код, используя Stream API. 4) Вынес метод Обновление полей в маппер, после внедрения MapStruct, генерируется автоматически. 5) Использовал stream api в методе getAll(). 6) Оптимизировал проверку уникальности email с помощью Set для достижения O(1) сложности. 7) Внедрил библиотеки MapStruct, теперь код маппинга генерируется автоматически и работает быстрее. 8) Добавил пустые строки для прохождения проверки Checkstyle.
|
|
||
| public UserServiceImpl(UserRepository userRepository) { | ||
| this.userRepository = userRepository; | ||
| userRepository.findAll().forEach(user -> emailSet.add(user.getEmail().toLowerCase())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не совсем понимаю зачем тут предзаполнять данные по email. Какая была идея?
Кажется, когда ты создаешь экземпляр UserServiceImpl, данные по пользователю все равно будут пустыми, пока пользователь не начнет пользоваться нашим API. А так как у нас пока хранение inmemory, при перезапуске приложения все данные будут стерты. Вполне достаточно добавлять email в сет при создании пользователя
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Убрал лишний код из UserServiceImpl)
| import ru.practicum.shareit.user.dto.UserDto; | ||
| import ru.practicum.shareit.user.model.User; | ||
|
|
||
| public class UserMapper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь тоже можно использовать mapstruct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Спасибо) Внедрил библиотеки MapStruct в UserMapper, теперь код стал более единообразным. Обновил тесты и конструктор.
1) Внедрил библиотеки MapStruct в UserMapper, теперь код стал более единообразным. 2) Обновил тесты и конструктор под MapStruct. 3) Убрал лишнее предзаполнение из UserServiceImpl.