>>1731888 (OP)ОП, проверь Вектор - https://ideone.com/06qnUR сделал с антикризисными мерами.Пробовал создавать новые объекты vector через сlone, не получалось (данные были одинаковы во всех остальных после применения антикризисных мер), пришел к сериализации.
>>1770074 →> Пробовал создавать новые объекты vector через сlone, не получалось Если ты клонируешь объект, содержащий другие объекты (например, клонируешь Департамент, в котором есть Работники), то основной объект клонируется, а вложенные - нет, просто в объект-клон копируются ссылки на них. Надо использовать магический метод __clone и в нем вручную клонировать вложенные объекты. Антикризисные меры логичнее сделать в отдельном объекте ("антикризисный комитет"). Так как у каждого класса должна быть своя зона ответственности, а в твоем примере Департамент отвечает и за управление списком работников, и за антикризисные меры. Дальше добавится еще задача, ты ее снова засунешь в Департамент, и получится класс-монстр, который занимается всем на свете.> dismissEmployee(int $id)Неудачно сделан аргумент - id - откуда мы его должны брать, он ведь не прописан в Работнике? Логичнее было бы сюда передавать объект-работника и пусть класс сам разбирается, как его найти.Увольнение инженеров лучше было бы сделать так: - выбрать всех инженеров (метод в департаменте)- отсортировать по рангу (usort)- отрезать первые 40% (array_slice)- уволить по получившемуся спискуТо есть тут можно использовать готовые функции работы с массивами вместо сложных циклов с кучей if внутри.> $job = get_class($search->getJob());> $search->setJob(new $job(), $maxRank, true);Тут стоило сделать метод changeRank() в Employee. Так как этот класс отвечает за хранение и обновление ранга, и код логичнее всего поместить в него. И $e->changeRank(...) проще читать, чем это. Неудачно сделан учет ранга. Логично иметь объект Job с базовыми (неизменными) ставками, и отдельно поле rank, и при расчете зарплаты учитывать ранг. А ты вместо этого меняешь сами базовые ставки в объекте Job. Если вызвать метод updateJob() несколько раз, то базовая ставка будет многократно увеличена: $emp->updateJob(3);$emp->updateJob(3);$emp->updateJob(3);То есть этот метод работает некорректно.
>>1792962- оставил сериализацию- сделал увольнение сотрудников по схожести объекта (Если один равен другому - удалить)- прилепил ко всему встроенные функции, избавился от кучи условий- зарплата считается на основе данных в классе профессии. (базовая ставка изменяется только для антикризисных мер)Теперь правда отличаются прошлые данные (с ideone) и текущие.Прочитал измененный код, вроде все должно быть хорошо =).https://github.com/Back1ng/vector
>>1794773Сериализация, это, конечно, костыльно, лучше бы ты разобрался с оператором clone и магическим методом __clone. Сериализация тебя может подвести, если в графе объектов есть циклические ссылки (объект A хранит в себе B, а B хранит в себе A, например, Работник хранит в себе ссылку на Департамент, а тот - на Работника).В комитете, мне кажется, логичнее было бы передавать Компанию в метод setAntiCrisisMeasuresFirst($vector), а не в конструктор. Тогда один Комитет мог бы оптимизировать хоть несколько Компаний подряд.> usort($engineers, function($a, $b) {> if ($a->isLeader() && $b->isLeader()) return 0;> if ($a->isLeader()) return 1;> return $a->getRank() <=> $b->getRank();> });Тут не хватает if ($b->isLeader()) { return -1 }. Или, что лучше, написать так: if ($a->isLeader() != $b->isLeader()) { return (int)$a->isLeader() <=> (int)$b->isLeader(); }.> array_map(function ($employee) use ($department) {Тут, наверно, читабельнее было бы использовать foreach, ведь array_map предназначен для преобразования массива в другой, и это только сбивает с толку. Не стоит использовать array_map как замену для foreach, стоит использовать его там, где мы преобразуем один массив в другой.Для замены босса было бы логично сделать в Департаменте метод replaceBoss($newBoss), а не руками переключать признак босса в цикле.inverseLeader() хуже читается, чем makeLeader()/demoteLeader(), в последних явно видно, какое значение мы ставим. Это неудачная функция, и трудно представить ситуацию, где нам нужно именно инвертировать признак босса, а не установить конкретное значение да/нет.> if (!($leader instanceof Analyst)) {Тут по моему ошибка: $leader->getJob(). На такой случай можно бы сделать метод $emp->isJob(Analyst::class).> * @param Department $departmentТип аргумента уже указан в тайп-хинте функции, незачем эту информацию дублировать, если в комментарии не содержится ничего нового. > public function getAverageExpenses()Тут можно было не дублировать код из функции getExpenses() , а вызвать ее.> @return Employee|falseЛучше возвращать null, тогда можно указать тайп-хинт ?Employee - Employee или null. (кстати, в новом PHP будут union types - можно писать func(): Employee | false ).> public function setRate(int $rate) : self> $this->rate = $rate;Тут у тебя ошибка - ты обращаешься к несуществующему в классе Job свойству. Нельзя обращаться к полям и методам, которые появятся только в наследниках. Так как когда кто-то будет наследовать твой класс, он может забыть добавить в него поля и методы и будет ошибка.Правильнее было бы сделать в Job абстрактные методы getBaseRate(), getBaseCoffee() и тд, а в наследниках их переопределять.На будущее, есть статические анализаторы кода вроде phpstan ( https://phpstan.org/ ), они покажут такие ошибки (и некоторые IDE тоже могут их находить). phpstan освоить несложно, достаточно скачать PHAR-файл и запустить из командной строки. Можешь попробовать освоить его. Поставь уровень побольше (опция -l). Также, есть статический анализатор psalm. В остальном, не так и плохо. Если захочется еще повозиться с ООП, можешь найти задачу про ООП-гостиницу. Думаю, навыки работы с ООП тебе помогут. когда будешь разбираться с фреймворками, хотя там конечно абстракции будут посложнее.
