implement method getSalaryInfo() to generate salary report - #1482
implement method getSalaryInfo() to generate salary report#1482KSORIGY wants to merge 6 commits into
Conversation
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.
…es and the report as well
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
REQUEST_CHANGES
Thank you for your submission! The core logic of your SalaryInfo implementation is correct and meets the main functional requirements. However, there are a couple of critical checklist violations that must be addressed before approval:
-
DateTimeFormatter Must Be a Constant
- The requirement states: "If a date formatter is created, it must be a constant field." Currently, the
DateTimeFormatteris instantiated inside the method. Please move it to aprivate static finalconstant field at the class level. This improves efficiency and adheres to the technical constraints. - Example:
private static final DateTimeFormatter DATE_FORMATTER = DateTimeFormatter.ofPattern("dd.MM.yyyy");
- The requirement states: "If a date formatter is created, it must be a constant field." Currently, the
-
Magic Numbers for Array Indices
- The indices used for
parts[](e.g.,parts[0],parts[1], etc.) are considered magic numbers. The checklist requires that such values be replaced with named constants for clarity and maintainability. - Please define constants like
INDEX_DATE,INDEX_NAME, etc., and use them instead of hardcoded numbers. - Example:
private static final int INDEX_DATE = 0; private static final int INDEX_NAME = 1; // ...
- The indices used for
-
(Minor) Redundant Empty Lines
- There are extra empty lines at the start of your class and method. While this is not a blocker, cleaning it up will improve code style.
Next Steps:
- Move the
DateTimeFormatterto a constant field. - Replace all magic numbers for array indices with named constants.
- Optionally, remove redundant empty lines for better style.
Once you address these points, your code will fully comply with all requirements. Great job on the core logic—just a few tweaks needed for full approval! If you have any questions about these changes, feel free to ask.
✨ 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 DateTimeFormatter should be declared as a private static final constant, not inside the method.
| int[] salary = new int[names.length]; | ||
|
|
||
| for (String record : data) { | ||
| String[] parts = record.split(" "); |
There was a problem hiding this comment.
This violates the checklist item: "Any magic numbers (hardcoded values with unclear meaning) must be moved to constant fields with informative names." The indices 0, 1, 2, 3 used for parts[] should be replaced with named constants.
|
|
||
| for (String record : data) { | ||
| String[] parts = record.split(" "); | ||
| LocalDate workDate = LocalDate.parse(parts[0], formatter); |
There was a problem hiding this comment.
This violates the checklist item: "Any magic numbers (hardcoded values with unclear meaning) must be moved to constant fields with informative names." The indices 0, 1, 2, 3 used for parts[] should be replaced with named constants.
| for (String record : data) { | ||
| String[] parts = record.split(" "); | ||
| LocalDate workDate = LocalDate.parse(parts[0], formatter); | ||
| String name = parts[1]; |
There was a problem hiding this comment.
This violates the checklist item: "Any magic numbers (hardcoded values with unclear meaning) must be moved to constant fields with informative names." The indices 0, 1, 2, 3 used for parts[] should be replaced with named constants.
| String[] parts = record.split(" "); | ||
| LocalDate workDate = LocalDate.parse(parts[0], formatter); | ||
| String name = parts[1]; | ||
| int hours = Integer.parseInt(parts[2]); |
There was a problem hiding this comment.
This violates the checklist item: "Any magic numbers (hardcoded values with unclear meaning) must be moved to constant fields with informative names." The indices 0, 1, 2, 3 used for parts[] should be replaced with named constants.
| LocalDate workDate = LocalDate.parse(parts[0], formatter); | ||
| String name = parts[1]; | ||
| int hours = Integer.parseInt(parts[2]); | ||
| int income = Integer.parseInt(parts[3]); |
There was a problem hiding this comment.
This violates the checklist item: "Any magic numbers (hardcoded values with unclear meaning) must be moved to constant fields with informative names." The indices 0, 1, 2, 3 used for parts[] should be replaced with named constants.
…es and the report as well
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
REQUEST_CHANGES
Your implementation is functionally correct and meets all core requirements for generating the salary report. However, there is a critical checklist violation that must be addressed before approval:
- Magic Numbers for Array Indices: Throughout your code, you use hardcoded numbers like
dataParts[0],dataParts[1], etc., to access elements in thedataPartsarray. According to the checklist, "Any magic numbers (hardcoded values with unclear meaning) must be defined as constant fields with informative names." This is important for code clarity and maintainability.
What you need to do:
- Define named constants for each index, such as:
private static final int INDEX_DATE = 0; private static final int INDEX_NAME = 1; private static final int INDEX_HOURS = 2; private static final int INDEX_INCOME_PER_HOUR = 3;
- Replace all usages of the hardcoded indices with these constants throughout your code.
Once you make this change, your submission will fully comply with all requirements. Great job on the rest of the implementation—you're very close to completion! Let me know if you have any questions or need clarification on this fix.
✨ 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
|
|
||
| for (String record : data) { | ||
| String[] dataParts = record.split(" "); | ||
| LocalDate workDate = LocalDate.parse(dataParts[0], DATE_FORMATTER); |
There was a problem hiding this comment.
This violates the checklist item: "Any magic numbers (hardcoded values with unclear meaning) must be defined as constant fields with informative names." The indices used for dataParts (e.g., dataParts[0], dataParts[1], etc.) should be replaced with named constants such as INDEX_DATE, INDEX_NAME, etc.
| for (String record : data) { | ||
| String[] dataParts = record.split(" "); | ||
| LocalDate workDate = LocalDate.parse(dataParts[0], DATE_FORMATTER); | ||
| String employeeName = dataParts[1]; |
There was a problem hiding this comment.
This violates the checklist item: "Any magic numbers (hardcoded values with unclear meaning) must be defined as constant fields with informative names." The indices used for dataParts (e.g., dataParts[0], dataParts[1], etc.) should be replaced with named constants such as INDEX_DATE, INDEX_NAME, etc.
| String[] dataParts = record.split(" "); | ||
| LocalDate workDate = LocalDate.parse(dataParts[0], DATE_FORMATTER); | ||
| String employeeName = dataParts[1]; | ||
| int workingHours = Integer.parseInt(dataParts[2]); |
There was a problem hiding this comment.
This violates the checklist item: "Any magic numbers (hardcoded values with unclear meaning) must be defined as constant fields with informative names." The indices used for dataParts (e.g., dataParts[0], dataParts[1], etc.) should be replaced with named constants such as INDEX_DATE, INDEX_NAME, etc.
| LocalDate workDate = LocalDate.parse(dataParts[0], DATE_FORMATTER); | ||
| String employeeName = dataParts[1]; | ||
| int workingHours = Integer.parseInt(dataParts[2]); | ||
| int incomePerHour = Integer.parseInt(dataParts[3]); |
There was a problem hiding this comment.
This violates the checklist item: "Any magic numbers (hardcoded values with unclear meaning) must be defined as constant fields with informative names." The indices used for dataParts (e.g., dataParts[0], dataParts[1], etc.) should be replaced with named constants such as INDEX_DATE, INDEX_NAME, etc.
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This is an educational approval to help you continue progressing in your learning journey!
What you did well:
- You fully implemented the
SalaryInfoclass and itsgetSalaryInfomethod according to the requirements. - All magic numbers (like array indices) are now defined as constants, which improves readability and maintainability.
- You used
LocalDateand a constantDateTimeFormatterfor date parsing, which is best practice for date handling in Java. - The code is clean, readable, and follows Java conventions, with descriptive variable and method names.
- Edge cases (such as employees with no records or records outside the date range) are handled correctly.
- No forbidden structures (like
Mapor 2D arrays) are used, and the code is self-contained.
Areas to keep in mind for future projects:
- Always define constants for any "magic numbers" or repeated values to make your code easier to understand and maintain.
- Continue to use descriptive names and consistent formatting—this makes your code much easier for others (and yourself) to read.
- Double-check that you handle all edge cases, as you did here, to ensure your code is robust.
Keep up the great work! Your attention to detail and willingness to address feedback are excellent habits. Use this project as a reference for future assignments, and keep challenging yourself to write clean, maintainable, and well-documented code. You're making strong progress!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
No description provided.