result by MerZakhar · Pull Request #1252 · mate-academy/jv-logger

MerZakhar · 2026-05-20T18:31:09Z

No description provided.

mateacademy-ai-mentor

Review Summary

This submission has critical violations of explicit checklist requirements that must be addressed. First, TODO comments remain uncompleted in AuthenticationServiceImpl.java (line 12) and OrderServiceImpl.java (lines 14, 31) — this violates [CHECKLIST ITEM #1] which requires all TODO comments to be removed. Second, log messages lack required parameter information: AuthenticationServiceImpl.java should log the login parameter (e.g., "Method login was called. Params: login={}"), and OrderServiceImpl.java should log userId parameter in both methods — this violates [CHECKLIST ITEM #3]. The log messages should be informative and include relevant method parameters. Additionally, there are Checkstyle violations for missing empty line separators before method definitions in both service files. Please complete the TODO comments, add parameter information to all log statements, and fix the spacing issues.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-05-20T18:32:43Z


 public class OrderServiceImpl implements OrderService {
+    private static final Logger logger = LogManager.getLogger(OrderServiceImpl.class);
    @Override


Checklist item #1 violation: This TODO comment should be completed. Add a log message like logger.info("Method login was called. Params: login={}", login);

mateacademy-ai-mentor · 2026-05-20T18:32:43Z

    @Override
    public Order completeOrder(Long userId) {
        // TODO: add log message about method completeOrder was called
+        logger.info("Method completeOrder was called");


Checklist item #3 violation: Log message should be informative. Consider adding the login parameter: logger.error("Username or password are incorrect. login={}", login);

mateacademy-ai-mentor · 2026-05-20T18:32:43Z

+    private static final Logger logger = LogManager.getLogger(OrderServiceImpl.class);
    @Override
    public Order completeOrder(Long userId) {
        // TODO: add log message about method completeOrder was called


Checklist item #1 violation: This TODO comment should be completed.

mateacademy-ai-mentor · 2026-05-20T18:32:43Z

@@ -25,6 +29,7 @@ private List<Product> getAllProductsFromShoppingCart(Long userId) {
        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


Checklist item #1 violation: This TODO comment should be completed.

mateacademy-ai-mentor · 2026-05-20T18:32:43Z

-        super(message);
+        logger.error(message);
    }
 }


TODO not completed. Per checklist item #1, all TODO comments must be removed. This TODO requires adding a log message about method login being called with parameters included (e.g., logger.debug("Method login was called. Params: login={}", login);)

mateacademy-ai-mentor · 2026-05-20T18:32:43Z

+    private static final Logger logger = LogManager.getLogger(AuthenticationServiceImpl.class);
    @Override
    public User login(String login, String password) throws AuthenticationException {
        //TODO: add corresponding log message about method login was called


Checklist #1 violation: Remove this TODO comment. The log statement should be added at the start of this method with the method parameters included (e.g., logger.debug("Method login was called. Params: login={}", login);)

mateacademy-ai-mentor · 2026-05-20T18:32:43Z

        User user = findByLogin(login);
        if (!user.getPassword().equals(password)) {
-            throw new AuthenticationException("Username or password are incorrect");
+            logger.error("Username or password are incorrect");


Checklist #3 violation: Log message should be informative and include parameters. Consider: logger.debug("Method completeOrder was called. Params: userId={}", userId);

mateacademy-ai-mentor

Review Summary

Your solution has critical violations of the task requirements that must be addressed before approval. First, the log4j2.xml configuration file with File and Console appenders is missing, which is explicitly required by Task requirement #1. Second, and most importantly, your log messages violate checklist item #3 which requires all log messages to include method parameters for informativeness.

In OrderServiceImpl.java, both logger.info() calls are missing the userId parameter. They should be updated to include parameters like: "Method completeOrder was called. Params: userId={}" and "Data from DB is fetched successful. Params: userId={}".

In AuthenticationServiceImpl.java, the error log is missing the login parameter. Update it to: logger.error("Method login was called. Params: login={}", login);

These are not cosmetic issues—log informativeness is an explicit checklist requirement. Once these parameters are added and the log4j2.xml configuration is provided, your submission will be ready for approval.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-05-20T18:39:21Z

    @Override
    public Order completeOrder(Long userId) {
-        // TODO: add log message about method completeOrder was called
+        logger.info("Method completeOrder was called");


This log message should include the login parameter as per checklist requirements. Example: logger.error("Method login failed. Params: login={}", login);

mateacademy-ai-mentor · 2026-05-20T18:39:21Z

    @Override
    public Order completeOrder(Long userId) {
-        // TODO: add log message about method completeOrder was called
+        logger.info("Method completeOrder was called");


This log message should include the userId parameter as per checklist requirements. Example: logger.info("Method completeOrder was called. Params: userId={}", userId);

mateacademy-ai-mentor · 2026-05-20T18:39:21Z

        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.info("Data from DB is fetched successful");


This log message should include the userId parameter as per checklist requirements. Example: logger.info("Data from DB is fetched successful. Params: userId={}", userId);

mateacademy-ai-mentor · 2026-05-20T18:39:21Z

        User user = findByLogin(login);
        if (!user.getPassword().equals(password)) {
-            throw new AuthenticationException("Username or password are incorrect");
+            logger.error("Username or password are incorrect");


This violates checklist item #3: log message should include the login parameter. Change to: logger.error("Method login was called. Params: login={}", login);

mateacademy-ai-mentor · 2026-05-20T18:39:21Z

        User user = findByLogin(login);
        if (!user.getPassword().equals(password)) {
-            throw new AuthenticationException("Username or password are incorrect");
+            logger.error("Username or password are incorrect");


This violates checklist item #3: log message should include the userId parameter. Change to: logger.info("Method completeOrder was called. Params: userId={}", userId);

mateacademy-ai-mentor · 2026-05-20T18:39:22Z

 public class AuthenticationServiceImpl implements AuthenticationService {
+    private static final Logger logger = LogManager.getLogger(AuthenticationServiceImpl.class);
+
    @Override


Missing log4j2.xml configuration file with File and Console appenders (Task requirement #1)

mateacademy-ai-mentor

Review Summary

This is an educational override to support your learning progress. Great work on fixing AuthenticationServiceImpl.java to include the login parameter and configuring .gitignore with the *.log pattern! There are still two minor issues in OrderServiceImpl.java that need attention: on line 20, the log message incorrectly says "Method login failed" but this is in the completeOrder method, so it should reference the actual method name instead; on line 32, the log is missing the userId parameter and should be logger.info("Data from DB is fetched successful. Params: userId={}", userId); per checklist item #3. Once you correct these two log messages, your solution will fully meet the requirements. Keep up the good work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2026-05-20T18:46:44Z

+        logger.info("Method login failed. Params: products={}, userId={}",
+                products, userId);


The log message says 'Method login failed' but this is the completeOrder method. Per checklist item #3, make the message informative by referencing the actual method: 'Method completeOrder was called. Params: userId={}'

mateacademy-ai-mentor · 2026-05-20T18:46:44Z

        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.info("Data from DB is fetched successful");


This log message is missing the userId parameter required by checklist item #3. Update to: logger.info("Data from DB is fetched successful. Params: userId={}", userId);

result

d375712

mateacademy-ai-mentor suggested changes May 20, 2026

View reviewed changes

MerZakhar added 2 commits May 20, 2026 21:35

fix

dcd87aa

fix2

737649c

MerZakhar requested a review from mateacademy-ai-mentor May 20, 2026 18:38

mateacademy-ai-mentor suggested changes May 20, 2026

View reviewed changes

fix3

9422a01

MerZakhar requested a review from mateacademy-ai-mentor May 20, 2026 18:45

mateacademy-ai-mentor approved these changes May 20, 2026

View reviewed changes

last

498b5f9

MerZakhar requested a review from mateacademy-ai-mentor May 20, 2026 18:59

		logger.info("Method login failed. Params: products={}, userId={}",
		products, userId);

Conversation

MerZakhar commented May 20, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants