Task is done! by Seekerfn · Pull Request #1269 · mate-academy/jv-logger

Seekerfn · 2026-07-28T15:26:53Z

No description provided.

mateacademy-ai-mentor

Review Summary

Your solution has critical violations of explicit checklist requirements that must be fixed before approval. TODO comments remain in multiple files (AuthenticationServiceImpl.java lines 14 and 16, OrderServiceImpl.java lines 16, 17, and 35) - checklist item #4 explicitly requires removing all TODO comments before submission. Additionally, the logger in AuthenticationServiceImpl.java is incorrectly initialized with OrderServiceImpl.class instead of its own class (AuthenticationServiceImpl.class), which breaks proper log categorization. Please remove all TODO comments and correct the logger class references to match their respective files.

✨ 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-07-28T15:28:13Z


 public class Main {
+
    public static void main(String[] args) {


Checklist item #4 violation: TODO comment still present - the comment on line 14 should be removed as the logging on the next line fulfills this requirement

mateacademy-ai-mentor · 2026-07-28T15:28:13Z

 import mate.academy.service.OrderService;
 import mate.academy.service.OrderServiceImpl;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;


Logger is initialized with wrong class. Should use AuthenticationServiceImpl.class instead of OrderServiceImpl.class for proper log categorization

mateacademy-ai-mentor · 2026-07-28T15:28:13Z

+
    public static void main(String[] args) {
+        Logger logger = LogManager.getLogger(OrderServiceImpl.class);
        AuthenticationService authenticationService = new AuthenticationServiceImpl();


Checklist item #4 violation: TODO comment still present - should be removed as the logging on the next line fulfills this requirement

mateacademy-ai-mentor · 2026-07-28T15:28:13Z

        } catch (AuthenticationException e) {
-            e.printStackTrace();
-            return;
+            logger.error("Got problem with user login: " + e);


Checklist item #3 violation: Error message uses string concatenation. Use parameter placeholder for better performance: logger.error("Got problem with user login", e)

mateacademy-ai-mentor · 2026-07-28T15:28:13Z

+
    @Override
    public User login(String login, String password) throws AuthenticationException {
        //TODO: add corresponding log message about method login was called


This violates checklist item #4: Remove all TODO comments before submitting solution. The TODO comment here is not needed since the log message is already implemented on line 15.

mateacademy-ai-mentor · 2026-07-28T15:28:13Z

+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;

 public class OrderServiceImpl implements OrderService {


Logger should use AuthenticationServiceImpl.class, not OrderServiceImpl.class

mateacademy-ai-mentor · 2026-07-28T15:28:13Z

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


Remove this TODO comment - the logging is already implemented on line 15

mateacademy-ai-mentor · 2026-07-28T15:28:13Z

+
        List<Product> products = getAllProductsFromShoppingCart(userId);
        Order order = new Order(products, userId);
        // NOTE: In production ready code this order identifier should be generated by DB


Use parameter placeholders instead of string concatenation: logger.error("Got problem with user login", e)

mateacademy-ai-mentor · 2026-07-28T15:28:14Z

+
    @Override
    public Order completeOrder(Long userId) {
        // TODO: add log message about method completeOrder was called


Remove this TODO comment - the logging is already implemented on line 17

mateacademy-ai-mentor · 2026-07-28T15:28:14Z

        Product xiaomi = new Product("Xiaomi 12", BigDecimal.valueOf(499));
        List<Product> products = List.of(iphone, macBook, xiaomi);
+        logger.info("Method successfully fetched data");
        // TODO: add log message about successful fetched data from DB


Remove this TODO comment

mateacademy-ai-mentor

Review Summary

Your submission has made progress by removing all TODO comments, but one critical issue remains unresolved that was explicitly flagged in previous reviews. In AuthenticationServiceImpl.java at line 10, the logger is still incorrectly initialized with LogManager.getLogger(OrderServiceImpl.class) instead of LogManager.getLogger(AuthenticationServiceImpl.class). This breaks proper log categorization — each service class should initialize its own logger with its own class reference, not another class's reference. This is checklist item #4 which currently fails. Please fix the logger initialization in AuthenticationServiceImpl.java by changing it to reference its own class. Once this is corrected, your implementation 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-07-28T15:34:48Z

 import mate.academy.service.OrderService;
 import mate.academy.service.OrderServiceImpl;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;


The logger is incorrectly referencing OrderServiceImpl.class instead of AuthenticationServiceImpl.class. This breaks proper log categorization and was flagged in the previous review. Change to: LogManager.getLogger(AuthenticationServiceImpl.class)

mateacademy-ai-mentor · 2026-07-28T15:34:48Z


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


Logger is incorrectly initialized with OrderServiceImpl.class instead of AuthenticationServiceImpl.class. This breaks proper log categorization - each class should have its own logger referencing itself.

mateacademy-ai-mentor · 2026-07-28T15:34:48Z

+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;

 public class OrderServiceImpl implements OrderService {


Logger is incorrectly referencing OrderServiceImpl.class instead of AuthenticationServiceImpl.class. Each class should initialize its logger with its own class for proper log categorization. Change to: LogManager.getLogger(AuthenticationServiceImpl.class)

mateacademy-ai-mentor

Review Summary

Your solution is approved! All previous critical issues have been resolved — AuthenticationServiceImpl.java now correctly initializes its logger with LogManager.getLogger(AuthenticationServiceImpl.class), and all other service classes follow the same pattern. No System.out.println() or e.printStackTrace() calls remain, and your .gitignore properly excludes the logs/ directory. Log messages are informative with proper {} parameterization. Note: log4j2.xml was listed as a changed file but wasn't included in the review context — please verify this configuration file exists with both File and Console appenders configured. This is an educational approval to help you progress while learning — the main implementation is solid and meets all checklist 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 ↩

Task is done!

3686ac2

mateacademy-ai-mentor suggested changes Jul 28, 2026

View reviewed changes

Task is done!2

ea47654

Seekerfn requested a review from mateacademy-ai-mentor July 28, 2026 15:33

mateacademy-ai-mentor suggested changes Jul 28, 2026

View reviewed changes

made a little change due to review

2b121f8

Seekerfn requested a review from mateacademy-ai-mentor July 28, 2026 15:47

mateacademy-ai-mentor approved these changes Jul 28, 2026

View reviewed changes


		private static final Logger logger = LogManager.getLogger(OrderServiceImpl.class);

		@Override


		public class AuthenticationServiceImpl implements AuthenticationService {

		private static final Logger logger = LogManager.getLogger(OrderServiceImpl.class);

Uh oh!

Conversation

Seekerfn commented Jul 28, 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!

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!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants