Add files via upload - #1481
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Sorry, the pull request review was unsuccessful for the following reasons:
Warning
Something went wrong while processing the review.
[CODE: GAE]
If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
REQUEST_CHANGES
Thank you for your submission! Your implementation of the SalaryInfo class and its core logic is correct and meets the main functional requirements. However, there are a few critical technical requirements that must be addressed before your code can be approved:
Critical issues to fix:
-
DateTimeFormatter must be a constant
- The requirement states: "If a date formatter is created, it must be a constant field." Currently, the
DateTimeFormatteris created inside the method. You need to move it to aprivate static finalfield at the class level, and name it using all uppercase letters with underscores (e.g.,DATE_FORMATTER). - This is important for both efficiency and code style compliance.
- The requirement states: "If a date formatter is created, it must be a constant field." Currently, the
-
Magic numbers for array indices
- The indices
0,1,2, and3used inparts[0],parts[1], etc., are considered magic numbers. The requirements specify that these should be replaced with named constants (e.g.,INDEX_DATE,INDEX_NAME, etc.). - This improves readability and maintainability.
- The indices
-
Redundant empty lines
- There are several redundant empty lines in your code. The requirements state: "Remove all redundant empty lines from the code." Please review your file and remove any unnecessary blank lines.
How to fix:
- Move the
DateTimeFormatterto a constant field at the top of your class. - Define named constants for each array index and use them instead of raw numbers.
- Clean up the code by removing extra empty lines.
Once you address these points, your code will fully comply with all requirements. The core logic is solid—just a few style and structure adjustments needed. Keep up the good work, and let me know if you have any questions!
✨ 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
| public class SalaryInfo { | ||
| public String getSalaryInfo(String[] names, String[] data, String dateFrom, String dateTo) { | ||
| return null; | ||
| DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd.MM.yyyy"); |
There was a problem hiding this comment.
This violates the requirement: "If a date formatter is created, it must be a constant field." The formatter should be declared as a static final constant, not inside the method.
| int totalSalary = 0; | ||
|
|
||
| for (String record : data) { | ||
| String[] parts = record.split(" "); |
There was a problem hiding this comment.
This violates the requirement: "Move any magic numbers to constant fields with informative names." The indices 0, 1, 2, and 3 used in parts[0], parts[1], etc., should be replaced with named constants.
|
|
||
| for (String record : data) { | ||
| String[] parts = record.split(" "); | ||
| String recordDateStr = parts[0]; |
There was a problem hiding this comment.
This violates the requirement: "Move any magic numbers to constant fields with informative names." The indices 0, 1, 2, and 3 used in parts[0], parts[1], etc., should be replaced with named constants.
| for (String record : data) { | ||
| String[] parts = record.split(" "); | ||
| String recordDateStr = parts[0]; | ||
| String recordName = parts[1]; |
There was a problem hiding this comment.
This violates the requirement: "Move any magic numbers to constant fields with informative names." The indices 0, 1, 2, and 3 used in parts[0], parts[1], etc., should be replaced with named constants.
| String[] parts = record.split(" "); | ||
| String recordDateStr = parts[0]; | ||
| String recordName = parts[1]; | ||
| int hours = Integer.parseInt(parts[2]); |
There was a problem hiding this comment.
This violates the requirement: "Move any magic numbers to constant fields with informative names." The indices 0, 1, 2, and 3 used in parts[0], parts[1], etc., should be replaced with named constants.
| .append(dateFrom) | ||
| .append(" - ") | ||
| .append(dateTo); | ||
|
|
There was a problem hiding this comment.
There is a redundant empty line here, which violates the requirement: "Remove all redundant empty lines from the code."
|
|
||
| for (String name : names) { | ||
| int totalSalary = 0; | ||
|
|
There was a problem hiding this comment.
There is a redundant empty line here, which violates the requirement: "Remove all redundant empty lines from the code."
| String recordName = parts[1]; | ||
| int hours = Integer.parseInt(parts[2]); | ||
| int rate = Integer.parseInt(parts[3]); | ||
|
|
There was a problem hiding this comment.
There is a redundant empty line here, which violates the requirement: "Remove all redundant empty lines from the code."
| totalSalary += hours * rate; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
There is a redundant empty line here, which violates the requirement: "Remove all redundant empty lines from the code."
| .append(" - ") | ||
| .append(totalSalary); | ||
| } | ||
|
|
There was a problem hiding this comment.
There is a redundant empty line here, which violates the requirement: "Remove all redundant empty lines from the code."
2 participants
No description provided.