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

Pull_reques_1 #1336

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

Conversation

RVoinahii
Copy link

Implement getSalaryInfo method for calculating and reporting employee salaries based on working hours. pay rates and specified date range.

… salaries based on working hours. pay rates and specified date range.
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 Pull Request! 👍 Just a couple of tips for the future: try to optimize algorithms when possible to avoid unnecessary loops, and remember that simplifying conditions can make your code cleaner. Also, consider defining reusable components, like formatters, as constants to improve maintainability. Keep it up! 💡

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

public class SalaryInfo {
public String getSalaryInfo(String[] names, String[] data, String dateFrom, String dateTo) {
return null;
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd.MM.yyyy");

Choose a reason for hiding this comment

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

Since the formatter is used multiple times in your code, it should be declared as a constant field. This will make your code cleaner and easier to maintain. Consider moving it to a static final field at the class level.

@@ -1,7 +1,49 @@
package core.basesyntax;

import java.time.LocalDate;
import java.time.format.DateTimeFormatter;

public class SalaryInfo {
public String getSalaryInfo(String[] names, String[] data, String dateFrom, String dateTo) {

Choose a reason for hiding this comment

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

Don't begin method implementation with an empty line. Please remove the empty line at the start of the method.

int dataHoursWorked = Integer.parseInt(partsOfData[2]);
int dataRatePerHour = Integer.parseInt(partsOfData[3]);

if (currentDate.isEqual(startDate) || currentDate.isAfter(startDate)) {

Choose a reason for hiding this comment

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

You can simplify the if-statement by using !currentDate.isBefore(startDate) instead of currentDate.isEqual(startDate) || currentDate.isAfter(startDate).

int dataRatePerHour = Integer.parseInt(partsOfData[3]);

if (currentDate.isEqual(startDate) || currentDate.isAfter(startDate)) {
if (currentDate.isEqual(endDate) || currentDate.isBefore(endDate)) {

Choose a reason for hiding this comment

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

Similarly, you can simplify the if-statement by using !currentDate.isAfter(endDate) instead of currentDate.isEqual(endDate) || currentDate.isBefore(endDate).


int[] employeesSalaries = new int[names.length];

for (String record : data) {

Choose a reason for hiding this comment

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

It's possible to optimize the algorithm by avoiding the nested loop. You can check if the date is within range first and then iterate only once through the names to update the salaries.

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