resolved the task by AndriiAlieksieienko · Pull Request #1241 · mate-academy/jv-logger

AndriiAlieksieienko · 2026-04-14T18:01:08Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because several core task requirements are not yet met, even though you correctly replaced System.out and printStackTrace with Log4j2 logging. The project is missing a visible Log4j2 configuration file (e.g. src/main/resources/log4j2.xml) that defines both a Console and a File appender writing into the logs/ directory (which is already ignored by .gitignore), so the main feature of the task is not fully implemented. In Main.java, OrderServiceImpl.java, and AuthenticationServiceImpl.java there are still TODO comments in methods like login and completeOrder that must be removed or completed as required by [CHECKLIST ITEM #1] and the task description. Many log messages are also too generic (for example "login method was called" or "method completeOrder was called") and should be made informative per [CHECKLIST ITEM #3] by including safe context such as login, userId, or product counts, e.g. logger.info("completeOrder called for userId={}", userId);. To fix this, 1) add a log4j2.xml (or properties) file with properly configured Console and File appenders pointing to logs/app.log, 2) remove/implement all remaining TODO markers in Main, OrderServiceImpl, and AuthenticationServiceImpl, and 3) update each log statement to include relevant parameters while avoiding sensitive data like passwords. Once these points are addressed, your solution will fully satisfy the functional and checklist requirements and will be in good shape.

✨ 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-14T18:03:02Z

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

 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 a Log4j2 configuration file (e.g. log4j2.xml or log4j2.properties) in the project. Add a configuration with Console and File appenders so logs are written where expected and the File appender can write into the logs directory excluded by .gitignore.

mateacademy-ai-mentor · 2026-04-14T18:03:02Z

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


Log message is too generic. The checklist requires: "Make log messages informative. [CHECKLIST ITEM #3]" Consider adding context (e.g. inputs or status) instead of "run main" — for example include args length or environment info.

mateacademy-ai-mentor · 2026-04-14T18:03:02Z

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


This error log could be more informative per the checklist: "Make log messages informative. [CHECKLIST ITEM #3]" Consider including which user/login failed (without logging the password) to help debugging: e.g. logger.error("Can't login for user={}", login, e);.

mateacademy-ai-mentor · 2026-04-14T18:03:02Z


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


There is an outstanding TODO in this class that wasn't removed/completed. The requirements state: "Remove all TODO comments before submitting solution. [CHECKLIST ITEM #1]" and "Complete all TODO statements" (description). Also the current log logger.info("login method was called") is not informative — follow checklist item #3 and include method parameters (avoid logging passwords).

mateacademy-ai-mentor · 2026-04-14T18:03:02Z

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


This method contains a remaining TODO that must be removed/completed: "Remove all TODO comments before submitting solution. [CHECKLIST ITEM #1]". Also logger.info("method completeOrder was called") is not informative per: "Make log messages informative. [CHECKLIST ITEM #3]" — include userId or other context in the log.

mateacademy-ai-mentor · 2026-04-14T18:03:03Z

        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("successful fetched data from DB");


A TODO comment was left here as well (remove all TODO comments: "Let's remove all TODO comments before submitting solution."). The log message should be more informative — include context such as userId and number of fetched products (e.g., "Fetched {} products for userId={}"). Also follow the checklist guidance: "Let's make our log messages informative."

mateacademy-ai-mentor · 2026-04-14T18:03:03Z

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


This startup log is minimal. Per checklist "Let's make our log messages informative" consider adding context (for example, include application args or environment info) while avoiding sensitive data.

mateacademy-ai-mentor · 2026-04-14T18:03:03Z

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

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


The task requires configuring the logger and adding appenders for File and Console (e.g., provide a log4j2.xml or programmatic configuration). I don't see any Log4j2 configuration file in the project. Please add a config with a Console and a File appender (and set the file path to a folder ignored by .gitignore).

mateacademy-ai-mentor · 2026-04-14T18:03:03Z

    @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");


A TODO comment remains in this class (see the preceding TODO line). This violates the checklist item: "Let's remove all TODO comments before submitting solution." Remove leftover TODO comments before submitting. Also the current log message is minimal — it doesn't include context/parameters. This violates the checklist item: "Let's make our log messages informative"; consider logging the login parameter, e.g. logger.info("login method was called. Params: login={}", login);

mateacademy-ai-mentor · 2026-04-14T18:03:03Z

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


The log message for completeOrder is not informative — it should include context such as the userId (or other relevant params). This violates the checklist item: "Let's make our log messages informative". Consider: logger.info("completeOrder called for userId={}", userId);

mateacademy-ai-mentor

Review Summary

I am requesting changes because the core logging requirement is not fully implemented yet: there is no visible Log4j2 configuration file that defines both a Console and a File appender writing to logs/app.log. The task explicitly requires configuring the logger with File and Console appenders, and currently src/main/java/mate/academy/Main.java, OrderServiceImpl.java, and AuthenticationServiceImpl.java all use Log4j2 without a corresponding log4j2.xml (or .properties) under src/main/resources, so file logging will not work.

Critical issues to fix (blockers):

Add a Log4j2 configuration file, e.g. src/main/resources/log4j2.xml, that defines:
- A Console appender (for example with a simple pattern layout).
- A File appender whose fileName is exactly logs/app.log so logs are written into the logs/ directory.
- A root logger that references both appenders so all existing logger calls in Main, OrderServiceImpl, and AuthenticationServiceImpl output to console and to the file.
Make sure you keep logs/ ignored in .gitignore (this is already set up with logs/*, so just ensure you do not commit the actual log files).

Minor notes: Your replacement of System.out.println and e.printStackTrace with logger calls and your informative log messages (including user IDs, logins, and product counts without logging passwords) are solid and do not need changes. Once you add the Log4j2 configuration file with the required appenders and correct path to logs/app.log, your solution will satisfy the main logging requirements for this task.

✨ 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-14T18:19:55Z

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

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


The project has logger usage here but there is no visible Log4j2 configuration file in the repository. Per the task description you must add a Log4j2 config (for example src/main/resources/log4j2.xml) that defines both a Console appender and a File appender writing to logs/app.log so that logs are written to the logs/ directory. Ensure the file appender path is logs/app.log and that logs/ remains ignored by .gitignore.

mateacademy-ai-mentor · 2026-04-14T18:19:55Z

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


The project uses Log4j2 but I don't see a Log4j2 configuration file (e.g. src/main/resources/log4j2.xml) that defines both a Console and a File appender. The task requires: "Configure the logger in this project. Add appenders for File and Console" and the previous review explicitly requested a visible Log4j2 config writing into the logs/ directory (e.g. logs/app.log). Please add a configuration file (log4j2.xml or properties) with a Console appender and a File appender pointing to logs/app.log so logging actually writes to console and the logs/ directory (which is already ignored by .gitignore).

mateacademy-ai-mentor · 2026-04-14T18:19:55Z

@@ -2,14 +2,18 @@

 import mate.academy.exception.AuthenticationException;
 import mate.academy.model.User;


This violates the project requirement: "Configure the logger in this project. Add appenders for File and Console". I cannot find a visible Log4j2 configuration (for example src/main/resources/log4j2.xml). Please add a Log4j2 config that defines both a Console appender and a File appender writing to logs/app.log and attaches them to the root logger. Note: .gitignore already contains logs/*, so the log file will be ignored as required.

mateacademy-ai-mentor

Review Summary

I am approving your solution as an educational override so you can move forward while continuing to practice proper Log4j2 configuration.

You did a nice job replacing System.out/printStackTrace with Log4j2 loggers in Main, AuthenticationServiceImpl, and OrderServiceImpl, and your messages are informative and parameterized. The main remaining requirement is to ensure that src/main/resources/log4j2.xml exists and defines both a Console appender and a File appender with fileName="logs/app.log", with the root logger referencing both of them so every log statement is written to the console and to that file. [CHECKLIST ITEM #1] Without that exact fileName and root logger wiring, the file-based logging part of the task is not fully satisfied even though the logger usage in code is correct. As you refine this, double‑check that the logs/ directory is created or auto-created by Log4j2, set append="true" on the file appender so logs are not overwritten, and keep logs/* in .gitignore so log files are never committed. As a polish step, also ensure every source file and configuration file ends with a newline character to match common style guidelines. Overall, you’re very close—your logging usage looks solid, and with a correctly wired log4j2.xml you’ll have a fully robust solution.

✨ 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-14T18:35:44Z

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

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


Logging is used here but the project lacks the Log4j2 configuration file. Add src/main/resources/log4j2.xml defining a Console appender and a File appender whose fileName is exactly "logs/app.log", and make the root logger reference both appenders so logs go to console and file.

mateacademy-ai-mentor · 2026-04-14T18:35:44Z

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


This class uses Log4j2, but without the required configuration file the File appender won't be created. Create src/main/resources/log4j2.xml with a Console appender, a File appender to "logs/app.log", and a root logger referencing both.

mateacademy-ai-mentor · 2026-04-14T18:35:44Z

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



This logger usage is fine, but as above you must add the Log4j2 config so file logging works. Ensure the File appender's fileName is exactly "logs/app.log" and the root logger references both Console and File appenders.

mateacademy-ai-mentor · 2026-04-14T18:35:44Z

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


The project is missing the Log4j2 configuration file that sets up Console and File appenders (so file logging to logs/app.log won't work). Please add e.g. src/main/resources/log4j2.xml that defines a Console appender and a File appender whose fileName is exactly logs/app.log, and a root logger that references both appenders. This addresses the requirement: "Configure the logger in this project and add appenders for File and Console so that logging goes to both destinations." Also follow the previous review note: "Add a Log4j2 configuration file, e.g. src/main/resources/log4j2.xml, that defines: - A Console appender. - A File appender whose fileName is exactly logs/app.log so logs are written into the logs/ directory. - A root logger that references both appenders so all existing logger calls output to both console and file."

mateacademy-ai-mentor · 2026-04-14T18:35:44Z

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


Critical: there is no Log4j2 configuration file in src/main/resources. Add a log4j2.xml that defines a Console appender and a File appender whose fileName is exactly "logs/app.log", and make the root logger reference both appenders so logs go to console and to logs/app.log.

mateacademy-ai-mentor · 2026-04-14T18:35:44Z

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



The class uses Log4j2 but without the required configuration file the File appender won't be created. Add src/main/resources/log4j2.xml (Console + File appenders, fileName="logs/app.log") so these logger calls also write to the file.

mateacademy-ai-mentor · 2026-04-14T18:35:44Z

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

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


This logger is fine in code, but the File appender is still missing at the project level. Create the required log4j2.xml under src/main/resources with a Console appender and a File appender pointing to logs/app.log and reference both from the root logger.

resolved the task

5814ab9

mateacademy-ai-mentor suggested changes Apr 14, 2026

View reviewed changes

improved the code

8fabfe9

AndriiAlieksieienko requested a review from mateacademy-ai-mentor April 14, 2026 18:17

mateacademy-ai-mentor suggested changes Apr 14, 2026

View reviewed changes

changed the log

80af0e0

AndriiAlieksieienko requested a review from mateacademy-ai-mentor April 14, 2026 18:33

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

View reviewed changes


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

		@@ -2,14 +2,18 @@

		import mate.academy.exception.AuthenticationException;
		import mate.academy.model.User;

Conversation

AndriiAlieksieienko commented Apr 14, 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants