Skip to content
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

дз #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

дз #12

wants to merge 1 commit into from

Conversation

munlite
Copy link
Owner

@munlite munlite commented Apr 13, 2020

No description provided.

Copy link

@Bael Bael left a comment

Choose a reason for hiding this comment

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

Из плюсов вашей работы:
Проделан огромный объем, чувствуется что вы подошли творчески.
Интересный подход с терминалом и авторизацией.
Из минусов - проверяйте апи который используется.
Если пишите много кода, есть шанс что часть его некорректна, поэтому или пишите тесты на него или проверьте сами.
Лучше начинать с небольшого решения которое потом обрастало бы функционалом. В вашем случае можно было бы поставить задачу как интерфейс продавца, который вносит покупку книги в базу. Тогда авторизация не нужна, а поиск по прежнему нужен, и логика проще.
Я сильно советую убрать rs.getObject методы, но на домашку они практически не влияют, это ваша экстра функциональность, поэтому задание принимаю.

*/
@Override
public boolean authorizationClient(Client client) {
if (clientRepository.findByFirstnameAndLastname(client.getFirstname(), client.getLastname()).isPresent()){
Copy link

Choose a reason for hiding this comment

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

можно было просто return clientRepository.findByFirstnameAndLastname(client.getFirstname(), client.getLastname()).isPresent()

*/
@Override
public long authenticationClient(Client client) {
return clientRepository.findByFirstnameAndLastname(client.getFirstname(), client.getLastname()).get();
Copy link

Choose a reason for hiding this comment

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

get на пустом Optional вызовет исключение

return jdbcTemplate.query("select id, author_id, book_id from author_of_book",
(rs, rowNum) -> AuthorOfBook.builder()
.id(rs.getLong("id"))
.author((Author) rs.getObject("author_id"))
Copy link

Choose a reason for hiding this comment

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

что то я сомневаюсь что это работает. jdbcTemplate это низкий уровень абстракции работающий с голым SQL, у него нет метаинформации что есть в Hibernate.
Так же смотрим документацию по этому методу:

This method will return the value of the given column as a Java object. The type of the Java object will be the default Java object type corresponding to the column's SQL type, following the mapping for built-in types specified in the JDBC specification.

т.е. вы получите Long объекты. По идее такой код должен падать в рантайме, вы не сталкивались с этим?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Вроде бы не сталкивался

Copy link
Owner Author

Choose a reason for hiding this comment

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

А что можно использовать вместо этого?

return jdbcTemplate.query("select id, id_book, id_client, price from deal",
(rs, rowNum) -> Deal.builder()
.id(rs.getLong("id"))
.client((Client) rs.getObject("id_client"))
Copy link

Choose a reason for hiding this comment

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

та же проблема с методом getObject

@Query("select SUM (d.price) from " +
"Deal d " +
"where d.book.title = ?1")
BigDecimal sumByBook(String title);
Copy link

Choose a reason for hiding this comment

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

Наблюдается увеличение количества методов под каждую хотелку, верно?
Возможно помог бы метод возвращающий поток Deal который вы могли бы использовать и для поиска автора, и сумм и книг через StreamAPI

.firstname(firstname)
.lastname(lastname)
.build();
if (asc.authorizationClient(client))
Copy link

Choose a reason for hiding this comment

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

а пароль? :)

Copy link
Owner Author

Choose a reason for hiding this comment

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

Просто решил сделать пока без праоля)

System.out.println("Книга куплена");
}
else {
System.out.println("Проверьте название книги");
Copy link

Choose a reason for hiding this comment

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

мы не знаем кто вызывает этот метод, поэтому полагаться на вывод на экран нельзя. Надо бросать исключение.

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.

2 participants