Домашняя работа 5

[Fatalll]

No description provided.

[ottergottaott]

В целом хорошо, но есть замечания, которые придется исправить.

• Role похожа на enum, но в данном виде это класс, в котором четыре интовых поля, даже не bool, т.е. у них могут быть какие угодно значения. Стоит приписать, что это enumeration.
• Помимо этого, кажется, что между Role должна быть другая связь, потому что Role не самостоятельная сущность и существовать в отрыве от User не может. В этом случае лучше подходит композиция. Более того, стрелка нарисована не в ту сторону, получается, что Role имеет доступ к User, а не наоборот, это касатеся остальных стрелок тоже.
• Кажется, что Guest не должен иметь имя, адрес и cardinfo, это избыточная для него информация, которая всегда будет фиктивной.
• Book содержит количество книг, цену и рейтинг. Эта информация, между делом, не статическая и будет изменяться в реальности, но ее никак не обновить в данном случае(не хватает сеттеров). И вообще кажется, что вести учет количества книг и рейтинга -- это все-таки не задачи самой книги.
• Почему Book принадлежит только одному каталогу? Хотелось бы, чтобы книги можно было представлять в разных каталогах, в рамках этой хотелки метод getCatalogID у Book выглядит как потекшая асбтракция и является лишним, поскольку книга не должна знать в каком каталоге лежит.
• Как представляется работа метода getCatalogById? Я не совсем понимаю что такое ID и какой Catalog ему может соответстовать.
• Метод addToBasket у WishList вряд ли добавляет весь WishList в корзину :) Было бы здорово из WishList предоставлять массив или какую-то другую коллекцию, по которой можно проитерироваться, и все. Пусть лучше WishList показывается пользователю, а потом от него приходит запрос на добавление отдельной книги в корзину из контроллера и обрабатывается OrderRepository.
• Метод paymentOrder у PaymentService тоже странный, что он принимает? Заказ и ID? Можно назвать его как-нибудь вроде payForOrder, станет чуть проще.
• Еще есть проблема с тем, что в PaymentService мы не знаем фиксированную стоимость заказа и ее придется считать. А что, если мы показали стоимость всего заказа пользователю, учитывая текущие цены на книги, отправили заказ оплачивать, за это время цены на книги поменялись и как следсвие к моменту оплаты сумма заказа возрасла, и с пользователя списалась сумма в полтора раза большая, чем заявлялось? Пользователь, наверное, зарустит. Тем временем в базе данных будет висеть заказ, фактическая оплата по которому не сходится ожидаемой оплатой, и тут загрустит не только пользователь, но и гендир.
• Еще было бы хорошо иметь ID для пользователя, а не идентифицировать его по userName и передавать везде String.
• FeedBackRepository выглядит немного недодуманным. Как добавляются отзывы? Почему addAuthoerFeedBack принимает FeedBack и не принимает id пользователя? И findApprovedFeedBacksForBook(Book) не возвращает ничего. Как происходит обновление отзыва? Мы для этого создаем новый отзыв? Хотелось бы увидеть эту часть завершенной.
• Очень хитрая сущность Database, представлена на слишком абстрактном уровне, но ок, можно простить.
• Про стрелки нужно почитать и правильно их развернуть. Например, Basket не композиция пользователей, а MainController не управляется репозиториями.
• Было бы хорошо сделать поля для классов, потому что сейчас поле есть только у Database
• Не обнаружил нигде пароли для пользователей.

[Fatalll]

• Role похожа на enum, но в данном виде это класс, в котором четыре интовых поля, даже не bool, т.е. у них могут быть какие угодно значения. Стоит приписать, что это enumeration.

Добавил, это действительно enum.

• Помимо этого, кажется, что между Role должна быть другая связь, потому что Role не самостоятельная сущность и существовать в отрыве от User не может. В этом случае лучше подходит композиция. Более того, стрелка нарисована не в ту сторону, получается, что Role имеет доступ к User, а не наоборот, это касатеся остальных стрелок тоже.

Подправил стрелки, вроде теперь ок должно быть.

• Кажется, что Guest не должен иметь имя, адрес и cardinfo, это избыточная для него информация, которая всегда будет фиктивной.

Перенес в RegisteredUser.

• Book содержит количество книг, цену и рейтинг. Эта информация, между делом, не статическая и будет изменяться в реальности, но ее никак не обновить в данном случае(не хватает сеттеров). И вообще кажется, что вести учет количества книг и рейтинга -- это все-таки не задачи самой книги.

Не предполагается её обновления (в частности в целях безопасности). Я считаю Book простой сущностью, обновление - запрос на новый экземпляр с тем же ID. Книжка просто является вьюшкой на информацию, которая лежит в БД.

• Почему Book принадлежит только одному каталогу? Хотелось бы, чтобы книги можно было представлять в разных каталогах, в рамках этой хотелки метод getCatalogID у Book выглядит как потекшая асбтракция и является лишним, поскольку книга не должна знать в каком каталоге лежит.

Забыл удалить, конечно же книжки могут быть в разных каталогах.

• Как представляется работа метода getCatalogById? Я не совсем понимаю что такое ID и какой Catalog ему может соответстовать.

Каталоги хранятся в БД, каждому из них сопоставлен уникальный ID. В частности, если нужно где-то на будущее сохранить каталог, достаточно сохранить его ID, заодно не придется беспокоиться об обновлении информации.

• Метод addToBasket у WishList вряд ли добавляет весь WishList в корзину :) Было бы здорово из WishList предоставлять массив или какую-то другую коллекцию, по которой можно проитерироваться, и все. Пусть лучше WishList показывается пользователю, а потом от него приходит запрос на добавление отдельной книги в корзину из контроллера и обрабатывается OrderRepository.

Вообще он именно это и делал х) Но да ладно, исправил на ваш вариант.

• Метод paymentOrder у PaymentService тоже странный, что он принимает? Заказ и ID? Можно назвать его как-нибудь вроде payForOrder, станет чуть проще.

PaymentService - представляет собой некоторую абстракцию над платежными системами, я с ними не работал, но уверен там куча всякой фигни, которую надо хранить, также логично делать его 1 на всю систему. Да, это как раз метод для оплаты, заказ и платежную информацию (я не представляю что нужно туда передать, поэтому просто String как универсальная заглушка).

• Еще есть проблема с тем, что в PaymentService мы не знаем фиксированную стоимость заказа и ее придется считать. А что, если мы показали стоимость всего заказа пользователю, учитывая текущие цены на книги, отправили заказ оплачивать, за это время цены на книги поменялись и как следсвие к моменту оплаты сумма заказа возрасла, и с пользователя списалась сумма в полтора раза большая, чем заявлялось? Пользователь, наверное, зарустит. Тем временем в базе данных будет висеть заказ, фактическая оплата по которому не сходится ожидаемой оплатой, и тут загрустит не только пользователь, но и гендир.

Добавил fixedPrice в заказ. Логика: цена - рассчитываемое поле, собирается из всех книг заказа на момент создания. Дальше пользователю дается некоторое время (для этого храним дату создания), чтобы его оплатить, если оплачивает - цена сохраняется, если нет, заказ считается отмененным.

• Еще было бы хорошо иметь ID для пользователя, а не идентифицировать его по userName и передавать везде String.

Этот String как раз таки ID, userName лежит в Feedback только для того, чтобы не делать каждый раз подзапрос на имя, т.к. я уверен, что он всегда показывается на сайте в отзывах.

• FeedBackRepository выглядит немного недодуманным. Как добавляются отзывы? Почему addAuthoerFeedBack принимает FeedBack и не принимает id пользователя? И findApprovedFeedBacksForBook(Book) не возвращает ничего. Как происходит обновление отзыва? Мы для этого создаем новый отзыв? Хотелось бы увидеть эту часть завершенной.

Хм, она додумана, видимо очень плохо описал :) Раскрыл Feedback. Идея - в MainController приходит запрос от формы на сайте, на создание/редактирование/удаление и пр. отзыва. Из полученных данных генерится FeedBack, если он уже есть, значит мы знаем его ID, а также ID пользователя - можем изменить, если нет и он новый, значит добавляем его в базу (объект уже готов, он просто без ID).

• Очень хитрая сущность Database, представлена на слишком абстрактном уровне, но ок, можно простить.

Но она как раз таки полная, ссылки на неё хранятся в каждом репозитории и они работают с ней на низком уровне (execQuery, isOpen). При старте открывается соединение, при выключении (?) закрывается.

• Про стрелки нужно почитать и правильно их развернуть. Например, Basket не композиция пользователей, а MainController не управляется репозиториями.

Да, стрелки пофиксил.

• Было бы хорошо сделать поля для классов, потому что сейчас поле есть только у Database

Вроде бы добавил.

• Не обнаружил нигде пароли для пользователей.

Они и не хранятся, в UserRepository есть метод validateUser, куда приходят учетные данные пользователя. По паролю берется хэш и отправляется запрос к БД на сравнение, после чего возвращается результат. ID = login, хотя лучше конечно так не делать, но я уже закоммитил х)

[yurii-litvinov]

• Не надо дублировать поля и ассоциации, например, в различных репозиториях, которые ссылаются на Database
• Не уверен, что seller-у и админу нужна корзина. Хотя это возможно
• Детали реализации типа геттеров для каждого поля, как в Order, на этой диаграмме лучше не рисовать, это мало что добавляет, зато ухудшает читаемость
• К enum-ам ассоциации обычно не рисуются, потому что они имеют семантику Value Object-ов

По существу, однако, всё правильно, так что зачтена