fork(6) download
copy 
  1. Давай для начала посмотрим на ошибки, а потом придумаем способ исправить. 
  2.  
  3. Ты решил сделать 1 класс, который обозначает Работника и 1 класс который 
  4. представляет Депарамент. Но при этом код, относящийся чисто к Работнику, 
  5. ты размазал по всей программе вместо того чтобы заключить его в одном классе. 
  6.  
  7. Например, расчет зарплаты: 
  8.  
  9. >  $k = $key > 0 ? ($key > 1 ? 1.5 : 1.25) : 1; 
  10.  
  11. помещен в функцию countDep() которая рассчитывает суммы по департаментам.
  12.  
  13. Допустим мы захотели дополнительно вывести поименный список всех работников 
  14. департамента продаж с указанием зарплаты каждого. Как это сделать в твоем коде,
  15. если расчет зарплаты засунут в функцию countDep()? Копипастить? Это одна из самых плохих вещей которые может сделать программист. 
  16.  
  17. Получается, с твоим подходом мы зайдем в тупик. Это говорит о том, что расчет 
  18. зарплаты надо было поместить в класс Работник, в этом случае мы можем из любого 
  19. места программы вызвать: 
  20.  
  21. зарплата = работник.посчитатьЗарплату();
  22.  
  23. Это было бы правильно, в стиле ООП.
  24.  
  25. Код расчета общей зарплаты по департаменту естественно должен быть в классе 
  26. Департамент.
  27.  
  28. Это были ошибки связанные с использованием ООП. Теперь пройдемся по обычным 
  29. ошибкам.
  30.  
  31. > public $meEmployees = array(0, 0, 0); 
  32. Ты на каждую профессию сделал отдельное свойство. Посчитай, в скольких 
  33. местах придется переделывать код, чтобы добавить новую профессию? В правильной 
  34. ООП-программе для этого достаточно было бы добавить 1 класс не трогая 
  35. существующие.
  36.  
  37. Также, я смотрю на эти 0, 0, 0 и не понимаю, что они значат. То есть сейчас я 
  38. догадываюсь что это ранги, но это абсолютно не очевидно и нули никак не 
  39. подписаны. Это плохо.
  40.  
  41. >  //array(type=>lvl)
  42. Это комментарии как бы намекают что код настолько запутанный что приходится 
  43. его комментировать.
  44.  
  45. Плюс, представь, мы захотим еще добавить, кроме ранга, опыт в годах и менять 
  46. зарплату в зависимости от опыта. много ли придется переделывать? Придется 
  47. переделывать всю программу. Это говорит о том, что она неправильно 
  48. спроектирована. Вот, как лучше сделать: 
  49.  
  50. Лучше сделать в классе Департамент поле-массив работников и соответственно 
  51. класть в него нужное число объектов-работников. Так каждому работнику будет 
  52. соответствовать один объект и мы можем хоть каждому индивидуально рассчитывать 
  53. зарплату, добавлять любое число рангов и правил, меняя только класс Работник 
  54. и не трогая весь остальной код. 
  55.  
  56. Далее. Для работников лучше сделать не 1, а 4 класса — свой для каждой профессии. При
  57. этом, понятно что у них будет много общего. Чтобы показать что эти классы
  58. связаны, и чтобы не копипастить ничего, надо применить тут наследование.
  59.  
  60. Наследование позволяет создавать класс не с нуля, а дополняя существующий. Мануал: 
  61. http://p...content-available-to-author-only...p.net/manual/ru/language.oop5.inheritance.php
  62.  
  63. Естественно, базовый класс надо будет пометить как абстрактный: 
  64. http://p...content-available-to-author-only...p.net/manual/ru/language.oop5.abstract.php
  65.  
  66. Если в мануале недостаточно понятно объяснено, задавай вопросы или почитай 
  67. соответствующие главы в книге Зандстры (название в ОП-посте).
  68.  
  69. > $advert->maEmployees[0] = 15;
  70.  
  71. Этот код абсолютно непонятен. Чтобы сделать его понятным надо либо записать
  72. его в виде $advert->addEmployees(15, 0) — в этом случае понятно станет из 
  73. названий переменных — либо сделать как я написал выше, массив работников. В данной задаче 
  74. подойдет второй вариант.
  75.  
  76. >  $result[0] = $obj->name;
  77. 0 ничего не говорит. Надо использовать понятные имена.
  78.  
  79. >  switch ($typeEmplCounter) {
  80. >            case "1":
  81. Это плохой код, так как "1" ничего не говорит. Что такое 1? Если тебе надо обозначать как-то
  82. типы объектов, надо использовать константы ( http://p...content-available-to-author-only...p.net/manual/ru/language.oop5.constants.php ): 
  83.  
  84. case Employee.TYPE_MANAGER: ...
  85.  
  86. Сравни, насколько понятнее. 
  87.  
  88. Ну и в этой задаче switch исплоьзовать недопустимо, надо просто сделать 4 класса для разных 
  89. профессий. При использовании switch код по мере развития быстро превращается в 
  90. запутанную лапшу.
  91.  
  92. Кстати, у тебя не учитываются расходы на босса.
  93.  
  94. >  foreach ($value as $word)
  95. >            $maxWidth = $maxWidth < (mb_strlen($word)) ? mb_strlen($word) : $maxWidth;
  96. Можно написать в 1 строку с помощью max() и array_map(), попробуй.
  97.  
  98. Ну и мы изучаем ООП, потому таблицу логично бы сделать объектом — с методами добавитьСтроку(), нарисовать(), задатьВыравниваниеДляКолонки().
  99.  
  100. Если что-то непонятно из того, что я написал, уточняй. Все же это неплохая задача, 
  101. сразу видно что человек понимает, а что нет.
  102.  
Success #stdin #stdout 0.01s 20520KB
comments (?)
stdin
copy 
Standard input is empty
stdout
copy 
Давай для начала посмотрим на ошибки, а потом придумаем способ исправить. 

Ты решил сделать 1 класс, который обозначает Работника и 1 класс который 
представляет Депарамент. Но при этом код, относящийся чисто к Работнику, 
ты размазал по всей программе вместо того чтобы заключить его в одном классе. 

Например, расчет зарплаты: 

>  $k = $key > 0 ? ($key > 1 ? 1.5 : 1.25) : 1; 

помещен в функцию countDep() которая рассчитывает суммы по департаментам.

Допустим мы захотели дополнительно вывести поименный список всех работников 
департамента продаж с указанием зарплаты каждого. Как это сделать в твоем коде,
если расчет зарплаты засунут в функцию countDep()? Копипастить? Это одна из самых плохих вещей которые может сделать программист. 

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

зарплата = работник.посчитатьЗарплату();

Это было бы правильно, в стиле ООП.

Код расчета общей зарплаты по департаменту естественно должен быть в классе 
Департамент.

Это были ошибки связанные с использованием ООП. Теперь пройдемся по обычным 
ошибкам.

> public $meEmployees = array(0, 0, 0); 
Ты на каждую профессию сделал отдельное свойство. Посчитай, в скольких 
местах придется переделывать код, чтобы добавить новую профессию? В правильной 
ООП-программе для этого достаточно было бы добавить 1 класс не трогая 
существующие.

Также, я смотрю на эти 0, 0, 0 и не понимаю, что они значат. То есть сейчас я 
догадываюсь что это ранги, но это абсолютно не очевидно и нули никак не 
подписаны. Это плохо.

>  //array(type=>lvl)
Это комментарии как бы намекают что код настолько запутанный что приходится 
его комментировать.

Плюс, представь, мы захотим еще добавить, кроме ранга, опыт в годах и менять 
зарплату в зависимости от опыта. много ли придется переделывать? Придется 
переделывать всю программу. Это говорит о том, что она неправильно 
спроектирована. Вот, как лучше сделать: 

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

Далее. Для работников лучше сделать не 1, а 4 класса — свой для каждой профессии. При
этом, понятно что у них будет много общего. Чтобы показать что эти классы
связаны, и чтобы не копипастить ничего, надо применить тут наследование.

Наследование позволяет создавать класс не с нуля, а дополняя существующий. Мануал: 
http://p...content-available-to-author-only...p.net/manual/ru/language.oop5.inheritance.php

Естественно, базовый класс надо будет пометить как абстрактный: 
http://p...content-available-to-author-only...p.net/manual/ru/language.oop5.abstract.php

Если в мануале недостаточно понятно объяснено, задавай вопросы или почитай 
соответствующие главы в книге Зандстры (название в ОП-посте).

> $advert->maEmployees[0] = 15;

Этот код абсолютно непонятен. Чтобы сделать его понятным надо либо записать
его в виде $advert->addEmployees(15, 0) — в этом случае понятно станет из 
названий переменных — либо сделать как я написал выше, массив работников. В данной задаче 
подойдет второй вариант.

>  $result[0] = $obj->name;
0 ничего не говорит. Надо использовать понятные имена.

>  switch ($typeEmplCounter) {
>            case "1":
Это плохой код, так как "1" ничего не говорит. Что такое 1? Если тебе надо обозначать как-то
типы объектов, надо использовать константы ( http://p...content-available-to-author-only...p.net/manual/ru/language.oop5.constants.php ): 

case Employee.TYPE_MANAGER: ...

Сравни, насколько понятнее. 

Ну и в этой задаче switch исплоьзовать недопустимо, надо просто сделать 4 класса для разных 
профессий. При использовании switch код по мере развития быстро превращается в 
запутанную лапшу.

Кстати, у тебя не учитываются расходы на босса.

>  foreach ($value as $word)
>            $maxWidth = $maxWidth < (mb_strlen($word)) ? mb_strlen($word) : $maxWidth;
Можно написать в 1 строку с помощью max() и array_map(), попробуй.

Ну и мы изучаем ООП, потому таблицу логично бы сделать объектом — с методами добавитьСтроку(), нарисовать(), задатьВыравниваниеДляКолонки().

Если что-то непонятно из того, что я написал, уточняй. Все же это неплохая задача, 
сразу видно что человек понимает, а что нет.
https://ideone.com/oZy19R
language:
PHP (php 7.3.5)
created:
10 years ago
visibility:
public

Share or Embed source code

Discover > Sphere Engine API

The brand new service which powers Ideone!

Discover > IDE Widget

Widget for compiling and running the source code in a web browser!