hw solution #1066
hw solution #1066
Conversation
There was a problem hiding this comment.
Thank you for your hard work on this pull request! 😊 There are a few critical issues that need to be addressed before we can approve it. The main concern is the missing logger configuration for File and Console appenders, which is a core requirement of the task. Additionally, while your log messages are generally informative, using parameterized logging instead of string concatenation will improve performance and readability. Please make these changes, and feel free to reach out if you have any questions. Keep up the great work! 🚀
✨ 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
| } catch (AuthenticationException e) { | ||
| e.printStackTrace(); | ||
| logger.error("AuthenticationException with incorrect data."); |
There was a problem hiding this comment.
The log message could be more informative. Consider including details about the exception or the parameters that caused the exception, as suggested in the checklist: logger.debug("Method login was called. Params: login={}, password={}", login, password);.
| @Override | ||
| public User login(String login, String password) throws AuthenticationException { | ||
| //TODO: add corresponding log message about method login was called | ||
| logger.debug("Method login was called. Params: login= " + login); |
There was a problem hiding this comment.
Use parameterized logging instead of string concatenation for better performance and readability. Change to: logger.debug("Method login was called. Params: login={}", login); as suggested in the checklist.
| @Override | ||
| public Order completeOrder(Long userId) { | ||
| // TODO: add log message about method completeOrder was called | ||
| logger.debug("Method completeOrder was called."); |
There was a problem hiding this comment.
Consider including the userId parameter in the log message for better traceability. For example: logger.debug("Method completeOrder was called. Params: userId={}", userId);.
| @@ -24,7 +28,7 @@ private List<Product> getAllProductsFromShoppingCart(Long userId) { | |||
| Product macBook = new Product("MacBook Air 2020", BigDecimal.valueOf(1399)); | |||
| Product xiaomi = new Product("Xiaomi 12", BigDecimal.valueOf(499)); | |||
| List<Product> products = List.of(iphone, macBook, xiaomi); | |||
| // TODO: add log message about successful fetched data from DB | |||
| logger.debug("Successful fetched data from DB."); | |||
There was a problem hiding this comment.
The log message could be more informative by including the userId parameter. Consider changing it to: logger.debug("Successfully fetched data from DB for userId={}", userId);.
There was a problem hiding this comment.
Great job on your submission! 🎉 While the overall structure is solid, there are a few critical areas to address to enhance your code:
-
Logger Configuration: Ensure that the logger is configured with appenders for both File and Console as per the task requirements. This is crucial for comprehensive logging.
-
Parameterized Logging: In the
Mainclass, replace string concatenation with parameterized logging for better performance and readability. For example, uselogger.error("Method login was called. Params: login = {}", login, e);. -
.gitignore: Your
.gitignorefile is well-configured to exclude unnecessary files, which helps keep the repository clean.
Remember, these improvements are essential for the functionality and maintainability of your code. Addressing these issues will not only help you meet the task requirements but also enhance your coding skills. Keep up the great work and continue learning from these experiences! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; |
There was a problem hiding this comment.
Ensure that the logger is configured with appenders for both File and Console as per the task requirements.
| public static void main(String[] args) { | ||
| AuthenticationService authenticationService = new AuthenticationServiceImpl(); | ||
| User user; | ||
| try { | ||
| user = authenticationService.login("bob", "1234"); | ||
| } catch (AuthenticationException e) { | ||
| e.printStackTrace(); | ||
| logger.error("Method login was called. Params: login = ", e); |
There was a problem hiding this comment.
Use parameterized logging instead of string concatenation for better performance and readability. For example: logger.error("Method login was called. Params: login = {}", login, e);.
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; |
There was a problem hiding this comment.
Ensure that the logger is configured with appenders for both File and Console as per the task requirements.
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; |
There was a problem hiding this comment.
Ensure that the logger is configured with appenders for both File and Console as per the task requirements.
No description provided.