Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented method getSalaryInfo #1333

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

svitlanagalicka
Copy link

No description provided.

Copy link

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

Choose a reason for hiding this comment

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

Great work on your getSalaryInfo method implementation! 👍 It's impressive to see that you've followed most of the checklist requirements, including the use of LocalDate and DateTimeFormatter, as well as StringBuilder for string concatenation. Just remember for the future to use System.lineSeparator() for cross-platform compatibility and consider more descriptive variable names. Keep up the good work, and always strive to make your code as clear and maintainable as possible! 😊👨‍💻

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

private static final int HOURS_INDEX = 2;
private static final int PER_HOUR_INDEX = 3;

public static String getSalaryInfo(String[] names, String[] data,

Choose a reason for hiding this comment

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

Remember to avoid beginning a method implementation with an empty line. In Java, it's a common practice to start the method body immediately after the method signature without an empty line to keep the code compact and readable.

LocalDate fromDate = LocalDate.parse(dateFrom, DATE_FORMATTER);
LocalDate toDate = LocalDate.parse(dateTo, DATE_FORMATTER);
StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append("Report for period ").append(dateFrom)

Choose a reason for hiding this comment

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

To ensure consistency across different operating systems, use System.lineSeparator() instead of hardcoded spaces when concatenating strings that are meant to be on separate lines.

for (String name : names) {
int salary = 0;
for (String info : data) {
String[] parts = info.split(" ");

Choose a reason for hiding this comment

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

Consider using more informative variable names. The name 'parts' is quite generic and does not convey the meaning of the data it holds. A name like 'recordParts' or 'workRecord' might be more descriptive.

salary += payPerHour * workedHours;
}
}
stringBuilder.append(System.lineSeparator()).append(name).append(" - ").append(salary);

Choose a reason for hiding this comment

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

Use System.lineSeparator() for consistency across different operating systems instead of relying on the default behavior of StringBuilder.append() which might not always use the correct line separator.

Comment on lines +29 to +31
if (employeeName.equals(name)
&& (workingDays.isAfter(fromDate) || workingDays.isEqual(fromDate))
&& (workingDays.isBefore(toDate) || workingDays.isEqual(toDate))) {
Copy link

Choose a reason for hiding this comment

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

think how you can shorten this to just two checks instead of for (use ! isBefore, ! isAfter)

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.

3 participants