Skip to content

Salary. First solution - #1478

Open
AliakseiS97 wants to merge 2 commits into
mate-academy:masterfrom
AliakseiS97:master
Open

Salary. First solution#1478
AliakseiS97 wants to merge 2 commits into
mate-academy:masterfrom
AliakseiS97:master

Conversation

@AliakseiS97

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

Thank you for your hard work! 😊 Your solution is functionally correct and well-structured, but there are two critical issues that need to be addressed before approval: (1) Please remove the unnecessary "Check data!" print statements, as the requirements specify that the method should not print anything except the result string; (2) Rename unclear variables like 'startMons' and 'endMonths' to more descriptive names such as 'startDate' and 'endDate'. Once you fix these, your code will fully meet the checklist requirements. Keep it up! 🚀


✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

int salary = 0;

for (String dataIndex : data) {
String dataStr = dataIndex.split(" ")[0];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: You are splitting the 'dataIndex' string multiple times for each field. According to the checklist, you should split it once and reuse the resulting array for better efficiency and clarity.

&& (!workingDay.isAfter(endMonths))) {
salary += Integer.parseInt(workHour) * Integer.parseInt(salaryPerHour);
} else {
System.out.println("Check data!");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: Printing 'Check data!' for every non-matching entry is not required by the task checklist and will clutter the output. You should remove this print statement.

.append(System.lineSeparator());

DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd.MM.yyyy");
LocalDate startMons = LocalDate.parse(dateFrom, formatter);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: The variable name 'startMons' is unclear and does not follow the checklist's recommendation for meaningful variable names. Consider renaming it to something like 'startDate'.


DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd.MM.yyyy");
LocalDate startMons = LocalDate.parse(dateFrom, formatter);
LocalDate endMonths = LocalDate.parse(dateTo, formatter);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: The variable name 'endMonths' is unclear and does not follow the checklist's recommendation for meaningful variable names. Consider renaming it to something like 'endDate'.

@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

This is an educational approval to help you continue progressing in your learning journey!

What you did well:

  • You fully met all the functional and technical requirements for the SalaryInfo class and its getSalaryInfo method.
  • Your code uses clear, descriptive variable and constant names, making it easy to read and maintain.
  • You correctly used LocalDate, DateTimeFormatter, and StringBuilder, and followed best practices like declaring constants and using System.lineSeparator().
  • Your test coverage is thorough, and your code handles input robustly, including malformed data.

Areas for future improvement:

  • Keep practicing the use of constants for all magic numbers and string literals—this is a great habit you demonstrated here!
  • Always double-check that your output format matches requirements exactly, including spacing and line breaks.
  • Continue to use informative names and robust error handling, as you did in this project.

Keep up the excellent work! Your attention to detail and adherence to best practices are impressive. Use this as a foundation for even more complex projects in the future!


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

Footnotes

  1. Rate AI review example

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