-
Notifications
You must be signed in to change notification settings - Fork 796
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
First solution #834
base: master
Are you sure you want to change the base?
First solution #834
Conversation
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.
Nice try! See some required changes and implement them:)
shoppingCartAnna.setUser(anna); | ||
shoppingCartAnna.setTickets(List.of(missionImpossible)); | ||
|
||
OrderService orderService = (OrderService) injector.getInstance(OrderService.class); |
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.
Let's also call getOrdersHistory before completing order.
public interface OrderDao { | ||
Order add(Order order); | ||
|
||
Order getByUser(User user); |
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.
One user can have multiple Orders.
Order getByUser(User user); | |
List<Order> getByUser(User user); |
Query<Order> query = session.createQuery("FROM Order o " | ||
+ "LEFT JOIN FETCH o.tickets t " | ||
+ "LEFT JOIN FETCH o.user " | ||
+ "LEFT JOIN FETCH t.movieSession ms " | ||
+ "LEFT JOIN FETCH t.user " | ||
+ "LEFT JOIN FETCH ms.movie " | ||
+ "LEFT JOIN FETCH ms.cinemaHall " | ||
+ "WHERE o.user =:user", Order.class); | ||
query.setParameter("user", user); | ||
return query.uniqueResult(); |
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.
Do not create redundant variables, you can set params and get result list directly on the session.createQuery()
return ..........Order.class).setParameter("user", user).getResultList();
@Inject | ||
private AuthenticationService authenticationService; | ||
@Inject | ||
private OrderDao orderDao; | ||
@Inject | ||
private UserDao userDao; |
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.
You should use these two here:
@Inject | |
private AuthenticationService authenticationService; | |
@Inject | |
private OrderDao orderDao; | |
@Inject | |
private UserDao userDao; | |
@Inject | |
private OrderDao orderDao; | |
@Inject | |
private ShoppingCartService shoppingCartService; |
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.
Good job!
.setParameter("user", user) | ||
.getResultList(); | ||
} catch (Exception e) { | ||
throw new DataProcessingException("Can't find a shopping cart by user: " + user, e); |
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.
The exception message is incorrect. It should say 'Can't find orders by user' instead of 'Can't find a shopping cart by user'.
order.setUser(shoppingCart.getUser()); | ||
order.setTickets(new ArrayList<>(shoppingCart.getTickets())); | ||
shoppingCartService.clearShoppingCart(shoppingCart); | ||
return orderDao.add(order); |
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.
The order is not saved in the database, so it can be lost after the method completeOrder is finished. You should add saving the order in the database.
private List<Ticket> tickets; | ||
private LocalDateTime orderDate; | ||
@ManyToOne(fetch = FetchType.LAZY) | ||
private User user; |
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.
The relationship between Order and User entities is not clearly defined. Consider adding 'cascade' attribute in @manytoone annotation and specify CascadeType as needed.
|
||
@Override | ||
public String toString() { | ||
return "Order{" |
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.
Including associated entities in the toString() method may lead to LazyInitializationException. Consider excluding 'tickets' and 'user' from toString().
@GeneratedValue(strategy = GenerationType.IDENTITY) | ||
private long id; | ||
@OneToMany(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST) | ||
private List<Ticket> tickets; |
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.
The relationship between Order and Ticket entities is not explicitly mapped. The 'mappedBy' attribute should be used in @onetomany annotation to specify the field that owns the relationship.
No description provided.