Skip to content

Implement SalaryInfo - #1583

Open
Vladyslav19ua wants to merge 1 commit into
mate-academy:masterfrom
Vladyslav19ua:master
Open

Implement SalaryInfo#1583
Vladyslav19ua wants to merge 1 commit into
mate-academy:masterfrom
Vladyslav19ua:master

Conversation

@Vladyslav19ua

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution because the core functionality is correct and all main task requirements are met: salary is calculated properly over the inclusive date range using LocalDate, DateTimeFormatter as a constant, nested loops, and StringBuilder with System.lineSeparator(). The report format matches the specification, and you respected the constraints of not using Map or multidimensional arrays. The remaining issues are minor style points: extra empty lines (e.g., after the package declaration, after the class declaration, and at the end of the file) [CHECKLIST ITEM #1], slightly vague variable names like datum and partsOfData that could be more descriptive, and the hardcoded string literal "Report for period " that should be moved to a static final constant [CHECKLIST ITEM #9]. These are important for clean code and passing style checks, but they are not functional blockers and can be fixed easily. After addressing those style refinements, your implementation will be fully polished—well done on the main logic and requirements.


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

Footnotes

  1. Rate AI review example

@@ -1,7 +1,46 @@
package core.basesyntax;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item "Don't begin class or method implementation with an empty line. Remove all redundant empty lines": there is an empty line between the package declaration and the first import, which counts as starting the class file with an unnecessary empty line.


public class SalaryInfo {
static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("dd.MM.yyyy");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item "Don't begin class or method implementation with an empty line. Remove all redundant empty lines": there is an extra empty line between the class declaration and the formatter field; the class implementation should not start with a blank line.


public String getSalaryInfo(String[] names, String[] data, String dateFrom, String dateTo) {
return null;
StringBuilder stringBuilder = new StringBuilder("Report for period "

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to checklist item "Any magic numbers should be constants / Move all hardcoded values to constant fields", the string literal "Report for period " should be extracted into a constant with an informative name instead of being inlined here.

return stringBuilder.toString();
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates checklist item "Don't begin class or method implementation with an empty line. Remove all redundant empty lines": there is an extra empty line at the end of the file after the closing brace of the class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants