Skip to content

Kursach1 is done#7

Open
ushki92 wants to merge 6 commits into
masterfrom
kursachcourse1
Open

Kursach1 is done#7
ushki92 wants to merge 6 commits into
masterfrom
kursachcourse1

Conversation

@ushki92

@ushki92 ushki92 commented Oct 9, 2021

Copy link
Copy Markdown
Owner

No description provided.

@DisterDE DisterDE left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Нужна существенная доработка.

public void setSalary(float salary) {
this.salary = salary;
}
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Comment on lines +13 to +19
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
EmployeeBook that = (EmployeeBook) o;
return Arrays.equals(employees, that.employees);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Книга у нас одна. Не думаю, что её будем с чем-то сравнивать.

StringBuilder print = new StringBuilder("Employees from department " + department + System.lineSeparator());
for (Employee employee : employees) {
if (employee != null && department == employee.getDepartment()) {
print.append(employee.getName()).append(" ").append(" ").append(employee.getSalary()).append(" ").append(employee.getId()).append(System.lineSeparator());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Вот чтобы это полотенце текста не писать, нужно реализовывать в Employee метод toString.
Тогда можно было бы написать append(employee) и всё.

}

public void changeEmployeeSalaryByName(String name, int newSalary) {
for (int i = 0; i < employees.length - 1; i++)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Скобки забыла.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Не могу найти где, вроде все работает

}
}
public void changeEmployeeDepartmentByName(String name, int newDepartment) {
for (int i = 0; i < employees.length - 1; i++)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Опять скобки забыла.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Всё ещё скобки.

}

public float departmentSalarySpendings(int department) {
float b = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Плохое имя.
totalSum или хотя бы result, если ничего в голову не лезет, но точно не b

float b = 0;
for (Employee employee : employees) {
if (employee != null && employee.getDepartment() == department) {
b = b + employee.getSalary();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

+=

Comment on lines +207 to +217
public float departmentAverageSalarySpendings (int department) {
float b = 0;
float c = 0;
for (Employee employee : employees) {
if (employee != null &&employee.getDepartment() == department) {
b = b + employee.getSalary();
c++;
}
}
return b/c;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

То же, что было сказано о прошлом методе.

Comment on lines +218 to +226
public void changeDepartmentSalaries(float incr, int department) {
float a;
for (Employee employee : employees) {
if (employee != null && employee.getDepartment() == department) {
a = employee.getSalary() + employee.getSalary() * (incr / 100);
employee.setSalary(a);
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

То же, что было сказано о прошлом методе по работе с зп.

Comment on lines +227 to +244
public String salaryToValueLess(float value) {
StringBuilder a = new StringBuilder();
for (Employee employee : employees) {
if (employee != null &&value > employee.getSalary()) {
a.append(employee.getName()).append(" ").append(employee.getSalary()).append(employee.getId()).append(System.lineSeparator());
}
}
return a.toString();
}
public String salaryToValueMoreOrEqual(float value) {
StringBuilder a = new StringBuilder();
for (Employee employee : employees) {
if (employee != null && value <= employee.getSalary()) {
a.append(employee.getName()).append(" ").append(employee.getSalary()).append(employee.getId()).append(System.lineSeparator());
}
}
return a.toString();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Здесь, как я помню, тоже не строку нужно возвращать.
Если я правильно помню, нужно было печатать тех, кто ниже по зп.

public void setSalary(float salary) {
this.salary = salary;
}
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@DisterDE DisterDE left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Всё ещё много исправлений. :)




public int counter;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Зачем это поле? Тем более публичное.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Поле поменяно на Privatе оно необходимо для ошибки если работник не найден

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

А еще оно выполняет подсчет числа работников

}

public void changeEmployeeSalaryByName(String name, int newSalary) {
int counter1 = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Эта переменная не требуется.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

counter1++;
}
}
if (counter1 > 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Если найдено несколько работников с одним ФИО, то это проблема.
Это не должно так работать.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Comment on lines +37 to +38
employees[i].setSalary(newSalary);
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Этот код должен быть в первом цикле, где происходит counter1++;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Переработано

employees[i].setSalary(newSalary);
return;
}
} else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

else не требуется вообще.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Comment on lines +216 to +221
for (int i = 0; i < employees.length - 1; i++) {
if (employees[i] != null && minSalary == employees[i].getSalary() && employees[i].getDepartment() == department) {
minSalaryDepartmentName = employees[i];
}

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Лишний цикл.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

for (int i = 0; i < employees.length - 1; i++) {
if (employees[i] != null && minSalary > employees[i].getSalary() && employees[i].getDepartment() == department) {
minSalary = employees[i].getSalary();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

minSalaryDepartmentName = employees[i];

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Comment on lines +225 to +241
public Employee findMaxSalaryDepartment(int department) {
Employee maxSalaryDepartmentName = null;
float maxSalary = 0;
for (Employee employee : employees) {
if (employee != null && maxSalary < employee.getSalary() && employee.getDepartment() == department) {
maxSalary = employee.getSalary();

}
}
for (Employee employee : employees) {
if (employee != null && maxSalary == employee.getSalary() && employee.getDepartment() == department) {
maxSalaryDepartmentName = employee;
}

}
return maxSalaryDepartmentName;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Те же комментарии, что были к прошлому методу.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

}
public float calculateDepartmentAverageSalarySpendings (int department) {
float totalSalary = 0;
float counter = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему этот счётчик float? У нас может быть 1,5 землекопа?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

return totalSalary/counter;
}
public void changeDepartmentSalaries(int percent, int department) {
float newSalary;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Эта переменная вне цикла не имеет смысла.
Её можно создавать там же, где идёт инициализация.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@DisterDE DisterDE left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Остались мелкие правки.

Comment on lines +14 to +18
public String toString() {
return name + ' ' + department +
" " + salary +
" " + id + System.lineSeparator();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Совсем не понимаю, почему id выводится последним.
И почему один из пробелов в виде char?

for (Employee employee : employees) {
if (employee != null && department == employee.getDepartment()) {
print.append(employee);
System.out.println(print);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Печать должна быть вынесена из цикла.

У тебя получается, что в консоль выводится каждый раз весь список сотрудников.

Т.е. добавила нового сотрудника в билдер, затем напечатала всё, что уже есть в билдере, затем добавила, затем снова напечатала весь билдер.

Печатать билдер нужно после цикла.

System.out.println("Not found Employee");
}

public void deleteEmployeeName(String name) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

byName

StringBuilder print = new StringBuilder("Employees " + System.lineSeparator());
for (Employee employee : employees) {
if (employee != null) {
print.append(employee);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Нужен ещё append '\n', так как сейчас они аппендятся в строку.

Comment on lines +204 to +206
float newSalary;
if (employee != null && employee.getDepartment() == department) {
newSalary = employee.getSalary() + employee.getSalary() * multiplier;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Зачем разделять объявление и инициализацию? Объедини строку с объявлением и инициализацией переменной в одну.

Comment on lines +211 to +228
public void printEmployeesWithSalaryToValueLess(float value) {
StringBuilder employeeLessToValue = new StringBuilder();
for (Employee employee : employees) {
if (employee != null && value > employee.getSalary()) {
employeeLessToValue.append(employee);
}
}
System.out.println(employeeLessToValue);
}
public void printEmployeesWithSalaryToValueMoreOrEqual(float value) {
StringBuilder employeeMoreToValue = new StringBuilder();
for (Employee employee : employees) {
if (employee != null && value <= employee.getSalary()) {
employeeMoreToValue.append(employee);
}
}
System.out.println(employeeMoreToValue);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Нет append новой линии между работниками.
При печати всё соберется в одну строку.

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