Complete task: add logger to main class by neuklearna · Pull Request #1240 · mate-academy/jv-logger

neuklearna · 2026-04-11T20:20:12Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because a core requirement of the task is not implemented and several explicit checklist items are violated.

Critical issues that must be fixed (blockers):

There is no Log4j2 configuration file (e.g., log4j2.xml or log4j2.properties under src/main/resources), so the required Console and File appenders are not configured as the task description demands. Add such a config that defines at least: a Console appender, a File appender writing to the already-ignored logs/ directory, and a root logger referencing both appenders at an appropriate level.
Several log messages are too generic and violate the requirement to make logs informative [CHECKLIST ITEM #3]. For example, in Main.java where you currently log logger.info("login");, and in AuthenticationServiceImpl and OrderServiceImpl where you log messages like "login", "Order is complete", or "All products in shopping cart", you should include key context such as login, userId, orderId, and counts (e.g., logger.info("Login attempt. login={}", login);, logger.info("Order completed. userId={} orderId={}", userId, order.getId());). Also, where you log exceptions (for example, in the catch block in Main), pass the exception object to the logger so the stack trace is recorded, e.g., logger.error("Failed to login. login={}", login, e);.

Minor improvements:

Review each log level and message in Main.java, OrderServiceImpl.java, and AuthenticationServiceImpl.java to ensure they carry enough context (IDs, counts, or names) and are at appropriate levels (INFO vs DEBUG).

Once you add the Log4j2 configuration with both appenders and make the log messages informative and exception-aware, your solution will align well with the task’s requirements.

✨ 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-04-11T20:22:08Z

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


The project uses Log4j loggers but I don't see a Log4j configuration that adds File and Console appenders. The task description explicitly requires: "Configure the logger in this project. Add appenders for File and Console". Add a log4j2 configuration (e.g. log4j2.xml or log4j2.properties under resources) registering a Console appender and a File appender (write files to the ignored logs/ folder).

mateacademy-ai-mentor · 2026-04-11T20:22:08Z

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


This logs only a short message without context. This violates the checklist item: "Let's make our log messages informative". Include meaningful context (for example: that login() was called and which login value was provided — do not log passwords), e.g. logger.info("login called. Params: login={}", login);.

mateacademy-ai-mentor · 2026-04-11T20:22:08Z

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


Message is too short and lacks context. This violates the checklist item: "Let's make our log messages informative". Include userId or order id in the message (e.g. logger.info("Order completed for userId={}. orderId={}", userId, order.getOrderId());).

mateacademy-ai-mentor · 2026-04-11T20:22:09Z

        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("All products in shopping cart");


This log statement is not informative (no user/product context). Per the checklist: "Let's make our log messages informative" — include relevant details such as userId, product count or product names: logger.info("All products in shopping cart for userId={}: {} items", userId, products.size());

mateacademy-ai-mentor · 2026-04-11T20:22:09Z


 public class Main {
+
+    private static final Logger logger = LogManager.getLogger(Main.class);


The task requires: "Configure the logger in this project. Add appenders for File and Console" (description). I don't see any Log4j2 configuration file (e.g. log4j2.xml or log4j2.properties) in the project — without it the appenders (File and Console) are not configured. Add a configuration file that defines Console and File appenders and appropriate loggers/levels (and ensure the log file path is inside the ignored logs/ directory).

mateacademy-ai-mentor · 2026-04-11T20:22:09Z


 public class Main {
+
+    private static final Logger logger = LogManager.getLogger(Main.class);


This violates checklist guidance: "Let's make our log messages informative" (checklist). logger.info("login"); is too terse and matches the bad example pattern. Consider including context such as the login parameter (but never log the password), e.g. logger.info("Login attempt. Params: login={}", login); and log success/failure with more details.

mateacademy-ai-mentor · 2026-04-11T20:22:09Z

+
+    private static final Logger logger = LogManager.getLogger(Main.class);
+
    public static void main(String[] args) {


This violates checklist guidance: "Let's make our log messages informative" (checklist). logger.info("Order is complete"); lacks context — include at least the userId and ideally the created orderId when available, e.g. logger.info("Order completed. userId={}. orderId={}", userId, order.getOrderId());.

mateacademy-ai-mentor · 2026-04-11T20:22:09Z

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


This violates the requirement: "Configure the logger in this project. Add appenders for File and Console" — I don't see a Log4j2 configuration file (e.g., log4j2.xml or log4j2.properties) in the project. Without it the appenders (Console and File) won't be configured. Add a configuration under resources with Console and File appenders and a logger/root logger configuration.

mateacademy-ai-mentor · 2026-04-11T20:22:09Z

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


This violates checklist item: "Let's make our log messages informative" — the message "login" is too short. Include context such as the username or operation details (e.g., logger.info("Login attempt: login={}", login)), and avoid generic single-word messages.

mateacademy-ai-mentor · 2026-04-11T20:22:09Z

-        //TODO: add corresponding log message about method login was called
+        logger.info("login");
        User user = findByLogin(login);
        if (!user.getPassword().equals(password)) {


This violates checklist item: "Let's make our log messages informative" — logger.info("Order is complete") lacks context. Include relevant info such as userId and orderId (when available), e.g. logger.info("Order completed: userId={} orderId={}", userId, orderId).

mateacademy-ai-mentor

Review Summary

I am requesting changes because some core logging requirements from the task are not yet met, so the solution is not fully compliant.

Missing Log4j2 configuration (critical blocker): There is no log4j2.xml or log4j2.properties under src/main/resources, but the task explicitly requires configuring a logger with both Console and File appenders; you should add a config file that defines at least a Console appender, a File appender writing into the already-ignored logs/ directory, and a root logger that references both at an appropriate level.
Exception logging loses stack traces (critical blocker): In src/main/java/mate/academy/Main.java, inside the catch block for login, you currently log only e.getMessage(), which drops the stack trace; update this to include the exception object and context, for example logger.error("Failed to login. login={}", login, e);, so failures can be properly diagnosed.
Log messages lack required context (important but easier fixes): In OrderServiceImpl and AuthenticationServiceImpl, some logs are emitted before the full context is available (e.g., logging an order as completed before orderId is set) or without key details like product counts; move the order-completed log so it runs after order.setOrderId(...) and include userId, orderId, and products.size(), and similarly update shopping-cart logs to include at least userId and the product count.
Login logs should be more informative: The login method logs in AuthenticationServiceImpl use terse wording; per the checklist, prefer an informative entry log such as logger.debug("Method login was called. Params: login={}", login); and a clear INFO-level message on successful login.

Your overall structure is good and you are using Log4j2 correctly in code; once you add the configuration file, fix the exception logging, and enrich/move the key log statements as described, your solution will satisfy the task requirements.

✨ 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-04-11T23:53:23Z

 import mate.academy.model.Product;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;



There is no Log4j2 configuration file under src/main/resources. The task description requires: "Configure the logger in this project. Add appenders for File and Console." Add a log4j2.xml or log4j2.properties that defines at least a Console appender, a File appender (writing to logs/), and a root logger referencing both.

mateacademy-ai-mentor · 2026-04-11T23:53:23Z

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


The login entry log includes the login but consider using a more informative message and the appropriate level. Per checklist "Let's make our log messages informative", prefer e.g. logger.debug("Method login was called. Params: login={}", login); (DEBUG for method entry) or logger.info("User login attempt. login={}", login); if you want it at INFO.

mateacademy-ai-mentor · 2026-04-11T23:53:23Z

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


This logs the order as "completed" before the orderId is set and without product count. To be informative (checklist: "Let's make our log messages informative"), either move/add a log after order.setOrderId(...) that includes orderId and number of products, e.g.: logger.info("Order completed. userId={} orderId={} productsCount={}", userId, order.getOrderId(), products.size());

mateacademy-ai-mentor · 2026-04-11T23:53:23Z

        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("All products in shopping cart for userId={}",userId);


Current message is generic. Per checklist "Let's make our log messages informative" include product count and/or product names, e.g.: logger.info("All products in shopping cart for userId={} productsCount={}", userId, products.size());.

mateacademy-ai-mentor · 2026-04-11T23:53:23Z

+    private static final Logger logger = LogManager.getLogger(AuthenticationServiceImpl.class);
+
    @Override
    public User login(String login, String password) throws AuthenticationException {


Project is missing a Log4j2 configuration file under src/main/resources that defines Console and File appenders (writing to the already-ignored logs/ directory) and a root logger referencing both appenders. This violates: "There is no Log4j2 configuration file (e.g., log4j2.xml or log4j2.properties under src/main/resources)... Add such a config that defines at least: a Console appender, a File appender writing to the already-ignored logs/ directory, and a root logger referencing both appenders at an appropriate level." Add e.g. src/main/resources/log4j2.xml with a Console appender, a File appender (file in logs/), and root logger referencing both.

mateacademy-ai-mentor · 2026-04-11T23:53:23Z

-        //TODO: add corresponding log message about method login was called
+        logger.info("Login called={}",login);
        User user = findByLogin(login);
        if (!user.getPassword().equals(password)) {


This log is emitted before the order id is assigned and doesn't include the order identifier or product count. The task requires informative logs with key context (orderId, counts). See: "Log messages should include key context (such as login, userId, orderId, and counts) to satisfy the requirement that logs be informative. [CHECKLIST ITEM #3]" Move or add the order-completed log after order.setOrderId(...) and include the orderId and product count (e.g. logger.info("Order completed. userId={} orderId={} productsCount={}", userId, order.getOrderId(), products.size());).

mateacademy-ai-mentor · 2026-04-11T23:53:23Z

    @Override
    public User login(String login, String password) throws AuthenticationException {
-        //TODO: add corresponding log message about method login was called
+        logger.info("Login called={}",login);


The login log uses terse wording Login called={}. The checklist asks to make logs informative: "* Let's make our log messages informative". Prefer a clearer message and include parameters like in the good example (e.g. logger.debug("Login attempt. Params: login={}", login);) and consider using DEBUG for method entry and INFO for successful login.

mateacademy-ai-mentor · 2026-04-11T23:53:23Z


 public class Main {
+
+    private static final Logger logger = LogManager.getLogger(Main.class);


Project is missing a Log4j2 configuration under src/main/resources that defines appenders and a root logger. This violates the requirement: "There is no Log4j2 configuration file (e.g., log4j2.xml or log4j2.properties under src/main/resources)... Add such a config that defines at least: a Console appender, a File appender writing to the already-ignored logs/ directory, and a root logger referencing both appenders at an appropriate level." Add a log4j2.xml (or properties) with a Console appender, a File appender writing into logs/, and a root logger referencing both.

mateacademy-ai-mentor · 2026-04-11T23:53:23Z

            user = authenticationService.login("bob", "1234");
        } catch (AuthenticationException e) {
-            e.printStackTrace();
+            logger.error(e.getMessage());


In the catch block you log only the exception message. The checklist requires you to pass the exception object so the stack trace is recorded: "Also, where you log exceptions (for example, in the catch block in Main), pass the exception object to the logger so the stack trace is recorded, e.g., logger.error("Failed to login. login={}", login, e);." Include the login context as well (capture the login in a variable before the try or log the literal) and pass e to the logger.

mateacademy-ai-mentor · 2026-04-11T23:53:23Z

+
+    private static final Logger logger = LogManager.getLogger(Main.class);
+
    public static void main(String[] args) {


This log message lacks full context required by the checklist: "Let's make our log messages informative" and you should "include key context such as login, userId, orderId, and counts". You're logging the order completion before the orderId is set and without product count. Move or add a log after the order is created and include orderId and number of products (for example: logger.info("Order completed. userId={} orderId={} productsCount={}", userId, order.getOrderId(), products.size());).

mateacademy-ai-mentor

Review Summary

I am approving your solution as an educational exception so you can progress while still having a couple of logging details to refine. You did well replacing System.out.println/e.printStackTrace, using Log4j2 consistently in Main and service classes, and including helpful context like userId, orderId, and product counts in your log messages. The most important remaining gap is that there is no Log4j2 configuration file under src/main/resources (e.g. src/main/resources/log4j2.xml), so Console/File appenders and the root logger are not defined; please add a config that declares a Console appender, a File appender writing into the logs/ directory, and a root logger that references both at an appropriate level (typically INFO). In AuthenticationServiceImpl.login (near the method body’s return statement, around lines 20–30), add an INFO-level log when login succeeds that includes both login and userId, so successful authentication events appear clearly in your normal logs. In the same method, fix the small typo in your debug message text (Parms → Params), and optionally tweak the error log in Main’s catch block so the message reads more clearly, such as Failed to login. login={} while still passing the exception object. Overall your logging structure and use of context are strong; once you add the config file and polish these messages, your implementation will align very well with the task’s requirements and good logging practices.

✨ 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-04-12T00:13:48Z

 import mate.academy.model.Product;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;



Missing Log4j2 configuration file under src/main/resources. The task requires adding a log4j2.xml or log4j2.properties that defines at least a Console appender, a File appender writing into the logs/ directory, and a root logger that references both appenders at an appropriate level. Without that config the File appender won't be created and file logging to logs/ will not work.

mateacademy-ai-mentor · 2026-04-12T00:13:48Z

+
    @Override
    public Order completeOrder(Long userId) {
-        // TODO: add log message about method completeOrder was called
        List<Product> products = getAllProductsFromShoppingCart(userId);
        Order order = new Order(products, userId);
        // NOTE: In production ready code this order identifier should be generated by DB
        // For test purpose we simplify this and return dummy data


AuthenticationServiceImpl.login does not log a successful login at INFO level. Per the requirements, add a clear INFO-level message on successful login that includes context (e.g. login and userId) before returning the user, for example: logger.info("User logged in successfully. login={}, userId={}", login, user.getUserId());

mateacademy-ai-mentor · 2026-04-12T00:13:48Z


 public class Main {
+
+    private static final Logger logger = LogManager.getLogger(Main.class);


Missing Log4j2 configuration file. The task requires: "Configure the logger in this project. Add appenders for File and Console" and additionally (from the detailed requirements) you must "Add a Log4j2 configuration file (log4j2.xml or log4j2.properties) under src/main/resources that: - Defines at least a Console appender. - Defines a File appender writing into the logs/ directory. - Defines a root logger that references both the Console and File appenders at an appropriate level." There is no file under src/main/resources configuring appenders/root logger. Add the config file so logs go to console and to logs/ (the logs/ directory is already ignored in .gitignore).

mateacademy-ai-mentor · 2026-04-12T00:13:48Z


 public class Main {
+
+    private static final Logger logger = LogManager.getLogger(Main.class);


Add an INFO-level log on successful login in AuthenticationServiceImpl.login. The task guidance asked: "In AuthenticationServiceImpl’s login method: Add an informative DEBUG-level entry log such as logger.debug("Method login was called. Params: login={}", login); Add a clear INFO-level log message on successful login." You already have a DEBUG entry (good), but there is no INFO log emitted when login succeeds—please add an informative info-level message (e.g. logger.info("User logged in. login={}, userId={}", login, user.getUserId()); ) before returning the user so successful auth events are recorded.

mateacademy-ai-mentor · 2026-04-12T00:13:48Z

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

 public class AuthenticationServiceImpl implements AuthenticationService {
+


The project is missing a Log4j2 configuration file under src/main/resources (e.g. log4j2.xml or log4j2.properties). The task requires a configuration that defines at least a Console appender and a File appender that writes into the logs/ directory, and a root logger referencing both appenders at an appropriate level. Please add such a config file so logging actually routes to console and the logs/ file.

mateacademy-ai-mentor · 2026-04-12T00:13:48Z

    @Override
    public User login(String login, String password) throws AuthenticationException {
-        //TODO: add corresponding log message about method login was called
+        logger.debug("Login attempt. Parms: login={}",login);


Add an INFO-level log for successful login and fix the small typo in the debug message. You already have an informative DEBUG entry which is good; also log success (for example: logger.info("User logged in successfully. login={}", login);). Also consider correcting Parms to Params in the existing debug message to keep messages clear.

Complete task: add logger to main class

9731285

mateacademy-ai-mentor suggested changes Apr 11, 2026

View reviewed changes

Add informative log messages with context parametrs

909a8b8

neuklearna requested a review from mateacademy-ai-mentor April 11, 2026 23:51

mateacademy-ai-mentor suggested changes Apr 11, 2026

View reviewed changes

Fix log messages: add context parameters and correct log

bdfdc1f

neuklearna requested a review from mateacademy-ai-mentor April 12, 2026 00:11

mateacademy-ai-mentor approved these changes Apr 12, 2026

View reviewed changes


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

		@Override


		public class Main {

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


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

		public static void main(String[] args) {

		import org.apache.logging.log4j.Logger;

		public class AuthenticationServiceImpl implements AuthenticationService {

Conversation

neuklearna commented Apr 11, 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants