feat: configure logging and replace println/stacktraces#1229
Conversation
4Empty4
commented
Mar 17, 2026
4Empty4
commented
- add log4j with console and rolling file appenders
- ignore generated log files in git
- replace println and printStackTrace with logger calls
- complete logging TODOs in services and main
- add log4j with console and rolling file appenders - ignore generated log files in git - replace println and printStackTrace with logger calls - complete logging TODOs in services and main
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work on implementing the logging feature! You've correctly configured Log4j, replaced system outputs, and handled exceptions as required, meeting all the core requirements. I am approving your solution.
For future improvement, consider making your log messages in AuthenticationServiceImpl.java more structured by including method parameters for easier debugging. Also, it's a good practice to avoid hardcoding values in log messages, like the username in Main.java, and to ensure files end with a newline. These are minor points, but they contribute to cleaner code. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| @Override | ||
| public User login(String login, String password) throws AuthenticationException { | ||
| //TODO: add corresponding log message about method login was called | ||
| logger.info("login() called for user: " + login); |
There was a problem hiding this comment.
This log message is informative, but it could be more structured to better align with the 'Good example' from the checklist. Consider including the parameter name in the message for clarity, for example: "login() method called with login='" + login + "'".
| logger.info("login() called for user: " + login); | ||
| User user = findByLogin(login); | ||
| if (!user.getPassword().equals(password)) { | ||
| logger.warn("Invalid credentials for user: " + login); |
There was a problem hiding this comment.
This warning message is good because it includes the username. To make it even more helpful, you could be more specific about why it failed. For example: "Invalid password attempt for user: " + login.
| logger.warn("Invalid credentials for user: " + login); | ||
| throw new AuthenticationException("Username or password are incorrect"); | ||
| } | ||
| logger.info("User authenticated: " + login); |
There was a problem hiding this comment.
This is a good log entry for a successful event. To improve consistency with other log messages, consider a more structured format like: "User authenticated successfully: " + login.
2 participants
