solved task jv-logger #1070
solved task jv-logger #1070
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this project! There are a few critical issues that need to be addressed before we can approve the pull request. These issues impact the core functionality and requirements of the task:
-
TODO Comments: There is an incomplete TODO comment in the
OrderServiceImpl.javafile. All TODOs must be completed as per the task requirements. -
Log Messages: Ensure all log messages are informative. Specifically, add a log message for successfully fetched data from the database in the
OrderServiceImpl.javafile. -
Logger Configuration: The logger needs to be configured with appenders for both File and Console, as required by the task.
-
Method Implementations: Verify that the
Userclass has the necessary methods likegetPassword()andgetUserId(), and ensure thefindByLoginmethod retrieves aUserobject correctly. -
Order Class Constructor: Ensure the
Orderclass has a constructor that accepts a list ofProductobjects and auserId.
Please address these issues to ensure the code meets all specified requirements. Keep up the great work, and I'm confident you'll have this ready in no time! 😊
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.
src/main/java/mate/academy/Main.java
Outdated
| public static void main(String[] args) { | ||
| AuthenticationService authenticationService = new AuthenticationServiceImpl(); | ||
| User user; | ||
| try { | ||
| user = authenticationService.login("bob", "1234"); | ||
| user = authenticationService.login("bob", "4124235"); |
There was a problem hiding this comment.
Ensure that the login method in AuthenticationServiceImpl is correctly implemented and that the User class has the necessary attributes and methods, such as getUserId(). This is crucial for the completeOrder method to function correctly.
| @Override | ||
| public User login(String login, String password) throws AuthenticationException { | ||
| //TODO: add corresponding log message about method login was called | ||
| logger.info("login method was called. Params: login = {}", login); | ||
| User user = findByLogin(login); |
There was a problem hiding this comment.
Ensure that the findByLogin method correctly retrieves a User object. Currently, it returns a dummy user with a hardcoded password. In a real application, this should query a database or another data source.
| @Override | ||
| public User login(String login, String password) throws AuthenticationException { | ||
| //TODO: add corresponding log message about method login was called | ||
| logger.info("login method was called. Params: login = {}", login); | ||
| User user = findByLogin(login); | ||
| if (!user.getPassword().equals(password)) { |
There was a problem hiding this comment.
Verify that the User class has a getPassword() method. This method is used to compare the provided password with the user's stored password.
| @Override | ||
| public Order completeOrder(Long userId) { | ||
| // TODO: add log message about method completeOrder was called | ||
| logger.info("method completeOrder was called. Params: userId = {}", userId); | ||
| List<Product> products = getAllProductsFromShoppingCart(userId); | ||
| Order order = new Order(products, userId); |
There was a problem hiding this comment.
Ensure that the Order class has a constructor that accepts a list of Product objects and a userId. This is necessary for creating an Order instance with the provided products and user ID.
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: 7]
If the issue is within your control, please address it.
The next review attempt will automatically launch in 1 hour.
If you believe this is an error or need assistance, please contact the support team.
There was a problem hiding this comment.
Review Summary
Great effort on your project! 🎉 While your code is on the right track, there are a few critical areas that need attention to fully meet the task requirements:
-
TODO Comments: There is an incomplete TODO comment in the
OrderServiceImpl.javafile. Please ensure to add a log message about successfully fetched data from the database as per the task requirements. -
Logger Configuration: It's crucial to configure the logger with appenders for both File and Console. This is a critical requirement that needs to be addressed.
-
Log Messages: Enhance the log messages to be more informative. For instance, in the
completeOrdermethod, consider adding more context about the operation being performed. -
Order Class Constructor: Ensure that the
Orderclass has a constructor that accepts a list ofProductobjects and auserId, as this is being used in theOrderServiceImpl.javafile. -
User Class Methods: Verify that the
Userclass has agetPassword()method and that it is implemented correctly, as it is being called in theAuthenticationServiceImpl.javafile. -
.gitignore Configuration: Your
.gitignorefile is well-configured to exclude the logs directory, aligning with the task requirements.
The decision to approve is made to help you move forward with your learning. Please review the comments and address these issues to improve your code. Remember, addressing critical issues is essential to avoid similar problems in the future. Keep up the good work and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| @@ -6,15 +6,19 @@ | |||
| import mate.academy.service.AuthenticationServiceImpl; | |||
| import mate.academy.service.OrderService; | |||
| import mate.academy.service.OrderServiceImpl; | |||
| import org.apache.logging.log4j.LogManager; | |||
There was a problem hiding this comment.
Ensure that the logger is configured with appenders for both File and Console as required by the task description .
| @Override | ||
| public User login(String login, String password) throws AuthenticationException { | ||
| //TODO: add corresponding log message about method login was called | ||
| logger.info("login method was called. Params: login = {}", login); | ||
| User user = findByLogin(login); |
There was a problem hiding this comment.
Verify that the User class has a getPassword() method, as this is being called here. Ensure that this method is implemented correctly.
| @Override | ||
| public Order completeOrder(Long userId) { | ||
| // TODO: add log message about method completeOrder was called | ||
| logger.info("method completeOrder was called. Params: userId = {}", userId); | ||
| List<Product> products = getAllProductsFromShoppingCart(userId); | ||
| Order order = new Order(products, userId); |
There was a problem hiding this comment.
Ensure that the Order class has a constructor that accepts a list of Product objects and a userId, as this is being used here.
No description provided.