Типа CodeReview - Githubissues

Привет! Напишу тут свой список вопросов и замечаний:

https://github.com/AnatoliyPostnov/LibraryNew/blob/26d11e975e74783ef9c0b1e3107223efc64d2f3e/readme.txt#L74 Эти методы нужно было по заданию реализовать или самому придумать, какие методы могут понадобиться? В частности, смущает путь для некоторых методов (например, тут для получения всех книг нужно указать id двух других книг?)
https://github.com/AnatoliyPostnov/LibraryNew/blob/26d11e975e74783ef9c0b1e3107223efc64d2f3e/src/main/java/com/postnov/library/service/OtherService/ConvertService.java#L7 Такие методы лучше прямо в DTO класс вставить, т.е. у каждого Dto класса будет свой метод from(), который будет из доменного класса делать Dto класс.
https://github.com/AnatoliyPostnov/LibraryNew/blob/26d11e975e74783ef9c0b1e3107223efc64d2f3e/src/main/java/com/postnov/library/service/OtherService/Impl/ConvertServiceImpl.java#L39 а вот тут modelMapper.map(notDto, classDto) - это какая-то хитрая штука от Spring, которая и делает преобразование из доменного в Dto класс?
https://github.com/AnatoliyPostnov/LibraryNew/blob/26d11e975e74783ef9c0b1e3107223efc64d2f3e/src/main/java/com/postnov/library/service/EntityService/impl/BookServiceImpl.java#L56 Тут и в похожих местах нужно работать с доменным объектом. Dto объекты нужны только для передачи данных и не нужно их тащить вглубь программы. Нужно либо (если сильно надо) в местах вызова этой функции преобразовывать dto в доменный, либо (так лучше) в функцию передавать только нужные параметры, а не весь объект.
https://github.com/AnatoliyPostnov/LibraryNew/blob/26d11e975e74783ef9c0b1e3107223efc64d2f3e/src/main/java/com/postnov/library/service/EntityService/impl/BookServiceImpl.java#L56 Там же цикл for можно заменить на стрим, получится короче и читабельнее, и вообще модно, стильно, молодёжно. Советую в целом посмотреть на Stream API, оно появилось в Java 8 и много где упрощает жизнь. Например, тут бы полуичлся такой код:
```
booksDto.stream()
            .filter(!bookRepository.findBookByNameAndVolume(bookDto.getName(), bookDto.getVolume()).isPresent())
            .forEach(bookDto -> authorService.saveAuthors(bookDto.getAuthors(), 
                    bookRepository.save(convertServiceBook.convertFromDto(bookDto, Book.class)).getId()));
```
https://github.com/AnatoliyPostnov/LibraryNew/blob/26d11e975e74783ef9c0b1e3107223efc64d2f3e/src/main/java/com/postnov/library/model/ReceivedBook.java#L7 Не уверен насчёт правильности выносить в отдельную сущность факт выдачи книги. Кажется, можно попробовать обойтись без отдельной сущности, и, например, привязывать книги к читательскому билету, и внутри LibraryCard сделать массив из Book.

Привет, Влад! Прокомментирую немного:

Задание звучало так: нужно реализовать ОРМ работника библиотеки. Все остальное придумывайте сами. Так вот в http://localhost:8080/books/by/fromBookId/and/toBookId?fromBookId={fromBookId}&toBookId={toBookId} я сделал, чтобы можно было получать все книги в диапазоне по айдишникам (fromBookId, toBookId). Ну да, наверно это немного странно, но на тот момент казалось классной идеей, чтобы не выдавать сразу миллион всех книг, которые есть в базе, выдать только часть, которая нужна клиенту... Наверное погинация тут была бы более уместна...
1. Классная идея, надо подумать над дальнейшим внедрением этой идеи в проект...
2. Ну да так и есть, взял отсюда: https://www.baeldung.com/entity-to-and-from-dto-for-a-java-spring-application
3. Замечание выглядит вполне логичным, если посмотреть на первый вариант проекта: https://github.com/AnatoliyPostnov/library я примерно так и делал. Первый вариант отличается от второго тем, что в первом варианте использовались аннотации типа @ManyToMany hibernate, а значит, имелись коллекции в Entity классах. Там я как раз и оперировал entity классами, а не DTO. Во втором же варианте, я писал все ручками. В Entity я хранил лишь айдишник другого entity - по сути внешний ключ. В Dto как раз хранились сами объекты. Так как выдать клиенту мне надо Dto, то требовалось написать определенную логику на формирование этого самого Dto, а логику в контроллерах иметь не очень хорошо, поэтому я вынес все это в сервисы... Сейчас примерно осознаю что эту логику можно засунуть в сами Dto, надо подумать над реализаций. Предполагаю, что возникнет вопрос, зачем отказываться от @ManyToMany. У разных программистов есть разные мнения, для кого-то использование этих возможностей hibernate хорошо, для кого-то нет. У каждого есть свои аргументы. Я решил сделать и так и так, чтобы попробовать написать по-разному...
4. Полностью согласен. Осваиваю этот метод. В последнем проекте, где я реализую микросервисную архитектуру https://github.com/AnatoliyPostnov/LibraryMicroservice, я старался использовать streamы и optional, пока это наверно выглядит немного криво, но продолжаю въезжать в эту тему. Пока конечно непривычный подход, но ничего.
5. С моей точки зрения мысль интересная и здравая, вначале как раз так сделать и хотел, но после некоторых размышлений остался на своем варианте. Это объясняется чисто логически. Мне показалось это решение более расширяемым. Типа Вот сервис который служит для получения книжки. А если я потом захочу купить эту книжку? Ну или еще что-то с ней сделать. В данной предметной области как-то сразу такое не приходит в голову, но я как-то на будущее смотреть пытаюсь. Лично мне как-то понятнее, когда под каждую бизнес функцию свой конкретный сервис желательно со своей базой. Тогда может нафиг LibraryCard? Но он тоже сюда напрашивается прям. В моей программе он особо ничего не делает, но возможно когда-нибудь понадобится, какие-то новые свойства хранить и тд и тп. В общем как-то пока так сделал. Очень благодарю, Влад, что потратил свое время, и прям достаточно глубоко вник в то, чего я тут понаписал. Все замечания для меня очень полезны, буду работать нас совершенствованием своих программерских скилов. Спасибо! Было бы круто, если бы нашел время глянуть https://github.com/AnatoliyPostnov/LibraryMicroservice. В части использования RabbitMQ И взаимодействия между микросервисами. Понятно, что там все просмотреть нужно прилично много времени...Спасибо!

AnatoliyPostnov / LibraryNew

Типа CodeReview #2