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

Homework for getiing thw sallary info #1382

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gekatonheyr
Copy link

implemented getSalaryInfo() method to calculate employee's salary for given period of time accordingly to quantity of working hours and hourly rate

… given period of time accordingly to quantity of working hours and hourly rate
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.


Thank you for your hard work on this pull request! 😊 While you've made some good progress, there are a few critical issues that need to be addressed before we can approve the changes:

  1. Date Handling: The current logic for date comparison is flawed. It checks if the record date is less than or equal to the start date and greater than or equal to the end date, which is incorrect. The correct logic should ensure that the record date is between the start date and end date inclusively. This is a critical issue as it directly impacts the functionality of the getSalaryInfo method. Please consider using LocalDate for more accurate and reliable date comparisons.

  2. Syntax Error: There's a missing semicolon at the end of a statement. This is a syntax error that needs to be fixed to ensure the code compiles correctly.

Once these issues are resolved, your code will be in great shape! Keep up the good work, and don't hesitate to reach out if you need any help. You're almost there! 🚀

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

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

@@ -2,6 +2,48 @@

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

Choose a reason for hiding this comment

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

The semicolon is missing at the end of the statement. Ensure to terminate statements with a semicolon.

Comment on lines 24 to 35
if ((Integer.parseInt(splittedRecordDate[2])
<= Integer.parseInt(splittedDateFrom[2])
&& (Integer.parseInt(splittedRecordDate[1])
<= Integer.parseInt(splittedDateFrom[1])
&& (Integer.parseInt(splittedRecordDate[0])
<= Integer.parseInt(splittedDateFrom[0]))))
|| (Integer.parseInt(splittedRecordDate[2])
>= Integer.parseInt(splittedDateTo[2])
&& (Integer.parseInt(splittedRecordDate[1])
>= Integer.parseInt(splittedDateTo[1])
&& (Integer.parseInt(splittedRecordDate[0])
> Integer.parseInt(splittedDateTo[0]))))) {

Choose a reason for hiding this comment

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

The date comparison logic is incorrect. The current logic checks if the record date is less than or equal to the start date and greater than or equal to the end date, which is logically flawed. You should check if the record date is between the start date and end date inclusively. Consider using a more straightforward date comparison approach or a date library to handle this.

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.

Review Summary

Great job on addressing the previous feedback and improving the code in SalaryInfo.java! 🎉

Key Improvements:

  • Date Handling: You've successfully corrected the logic for date comparison. The use of LocalDate ensures accurate and inclusive checks between the start and end dates.
  • Syntax Error: The missing semicolon issue has been resolved, and the code now compiles without syntax errors.

Your solution now meets the task requirements and constraints. However, it's important to always double-check for syntax errors, like missing semicolons, to ensure smooth compilation.

Moving Forward:

While the current code is approved, remember to review the comments and address any minor issues to enhance your coding skills. This will help you avoid similar problems in the future and improve the overall quality of your code.

Keep up the great work and continue learning from these experiences! 🚀

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

LocalDate ldRecordDate = LocalDate.parse(splittedEmployeeRecord[0], FORMATTER);
if ((ldRecordDate.equals(ldDateFrom) || ldRecordDate.isAfter(ldDateFrom))
&& (ldRecordDate.equals(ldDateTo) || ldRecordDate.isBefore(ldDateTo))) {
totalEmployeeSalary += Integer.parseInt(splittedEmployeeRecord[2])

Choose a reason for hiding this comment

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

There is a missing semicolon at the end of this line. This is a syntax error that needs to be fixed to ensure the code compiles correctly.

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