«phpClub» — архив тем ("тредов"), посвящённых изучению PHP и веб-технологий.
Аноним 2019/04/19 12:40:04  №1384130 1
Опчик, можешь глянуть 1 задачу по ООП? 200 строк это нормально, или я его сильно раздул и нужно пытаться писать более сжато?
https://github.com/Leorne/VektorCompany/blob/master/Vektor.php
Почему когда я делаю свойства класса Position - private (33 строка), то методы которые должны возвращать эти значения возвращают 0? (Все кроме getTotalEmployees, с ним всё ок)
Аноним 2019/04/21 00:35:38  №1385176 2
>>1384130

Когда ты столкнешься с реальными проектами, ты поймешь, что 200 строк - это ерунда. На практике каждый класс помещают в отдельный файл.

Код у тебя по моему не соответствует форматированию PSR, например, пробелы вокруг "равно" хаотично расставлены. Если ты будешь использовать IDE, то там код форматируется одной кнопкой. Также, см. второй пост треда про другие способы отформатировать код.

> private $nameOfDepartment;
Достаточно просто $name. В данном случае удлинение имени переменной не дает никакой новой информации.

> public function setPosition(string $name,int $countOfPerson,int $tier =1,int $leader = 0){
Этот метод содержит захардкоженный список профессий и не позволит добавить новые, не меняя старый код. Лучше убрать создание работников из Департамента и принимать на вход уже созданного работника. Департамент ведь нанимает людей, а не создает.

Комментарии лучше писать сверху, а не справа, а то строки получаются длинные.

> abstract class Position{
>\tpublic $payment;
Лучше использовать protected, а то у тебя можно без всяких проверок записать что угодно в эти поля снаружи. То есть сделай инкапсуляцию: доступ к свойствам снаружи есть только через методы, которые не позволят задать некорректные значения.

> function __construct(int $countOfPerson,int $tier,int $leader){
У тебя один объект Position представляет целую группу работников. Это неудобно, так как ты не можешь одному из этой группы повысить ранг, например. В части задачи про кризис это тебе помешает.

> \t\tif ($tier != 1){
>\t\t $this->payment += ($this->payment($tier0.25-0.25));
Здесь ты смешал в одном поле 2 разных значения: базовую ставку и итоговую зарплату.

Также, у твоего объекта работника есть серьезный недостаток: зарплата считается только при создании. Если мы повысим работнику ранг, зарплата не обновится. А по идее зарплата должна обновляться при изменении любого влияющего на нее свойства (подсказка: проще всего ее вообще не хранить).

> public function getTotalPayment(){
Стоит указывать тип возвращаемого функцией значения.

> class Manager extends Position{
>\tpublic $payment = 500;
У тебя класс-наследник должен указать базовую ставку для профессии. Но это никак не описано в коде и никак не проверяется. Легко забыть это сделать. Лучше объявить абстрактные методы вроде getBaseSalary, тогда не определить их в наследнике не получится.

> $sellDepartment->setPosition('manager',12);
Вместо названия лучше было бы использовать встроенную константу, возвращающую имя класса: Marketer::class. Так при опечатке будет выдана ошибка. У тебя же опечатка будет молча проигнорирована, что очень плохо - это не позволяет сразу увидеть ошибку и придется потратить много времени на отладку.
Ответы: >>1387082
Аноним 2019/04/22 21:47:45  №1387082 3
>>1385176
>Когда ты столкнешься с реальными проектами, ты поймешь, что 200 строк - это ерунда.

Да я спрашиваю к тому, чтобы знать: раздут мой код или нет. Если можно сделать в 100 строк, а я сделал в 400 - значит я написал не так хорошо как мог

>Лучше убрать создание работников из Департамента и принимать на вход уже созданного работника.Департамент ведь нанимает людей, а не создает.

Сделать в департаменте массив из работников, который будет содержать в себе все виды работников? Не совсем понял где тогда создавать работников, ведь я вызывал метод
setPosition и устанавливал все необходимые входные данные там. С названием "должности" я понял проблему, это можно починить передавая константы с именем класса. Если константы не существует - значит и класса такого нет, а значит и нет проблем.

>Лучше использовать protected, а то у тебя можно без всяких проверок записать что угодно в эти поля снаружи.

Правильно ли я понимаю, что желательно для всех свойств класса ставить protected/private, а для считывания/записи нужно делать public методы с проверкой и выдачей ошибки в случае чего?

>У тебя один объект Position представляет целую группу работников. Это неудобно, так как ты не можешь одному из этой группы повысить ранг, например. В части задачи про кризис это тебе помешает.

Неужели для каждого рабочего свой обьект создавать? Ранг повысить я смог, если нужно повысить 4 человека с 1 ранга до 2, то с группы людей 1 ранга(кол-ва человек) отнимаем 4, и создаем новый обьект в департаменте с рангом +1 (с кол-вом человек = 4 ).

>Вместо названия лучше было бы использовать встроенную константу, возвращающую имя класса: Marketer::class.

Не совсем понял. Обьявить в начале кода 4 константые Manager,Engineer и т.д?

>Здесь ты смешал в одном поле 2 разных значения: базовую ставку и итоговую зарплату.

Т.е базовая ставка у всех одинаковая, а итоговая зависит от ранга и является ли работник лидером?

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

ОП, спасибо тебе огромное что уделил время
Ответы: >>1387094 >>1393246 >>1394576
Аноним 2019/04/22 22:12:15  №1387094 4
>>1387082
>Если можно сделать в 100 строк, а я сделал в 400 - значит я написал не так хорошо как мог
главное какой вариант легче читать и поддерживать