Почему мой класс хуже, чем иерархия классов в книге (начинающий ООП)?

11

Я читаю PHP объекты, шаблоны и практики . Автор пытается смоделировать урок в колледже. Цель состоит в том, чтобы вывести тип урока (лекция или семинар), а также плату за урок в зависимости от того, является ли это почасовым или фиксированным уроком. Таким образом, вывод должен быть

Lesson charge 20. Charge type: hourly rate. Lesson type: seminar.
Lesson charge 30. Charge type: fixed rate. Lesson type: lecture.

когда вход выглядит следующим образом:

$lessons[] = new Lesson('hourly rate', 4, 'seminar');
$lessons[] = new Lesson('fixed rate', null, 'lecture');

Я написал это:

class Lesson {
    private $chargeType;
    private $duration;
    private $lessonType;

    public function __construct($chargeType, $duration, $lessonType) {
        $this->chargeType = $chargeType;
        $this->duration = $duration;
        $this->lessonType = $lessonType;
    }

    public function getChargeType() {
        return $this->getChargeType;
    }

    public function getLessonType() {
        return $this->getLessonType;
    }

    public function cost() {
        if($this->chargeType == 'fixed rate') {
            return "30";
        } else {
            return $this->duration * 5;
        }
    }
}

$lessons[] = new Lesson('hourly rate', 4, 'seminar');
$lessons[] = new Lesson('fixed rate', null, 'lecture');

foreach($lessons as $lesson) {
    print "Lesson charge {$lesson->cost()}.";
    print " Charge type: {$lesson->getChargeType()}.";
    print " Lesson type: {$lesson->getLessonType()}.";
    print "<br />";
}

Но, согласно книге, я ошибаюсь (я тоже почти уверен). Вместо этого автор дал большую иерархию классов в качестве решения. В предыдущей главе автор указал следующие «четыре указателя» как время, когда мне следует подумать об изменении структуры моего класса:

  • Дублирование кода
  • Класс, который слишком много знал о его контексте
  • Мастер на все руки - классы, которые пытаются делать много вещей
  • Условные заявления

Единственная проблема, которую я вижу, - это условные утверждения, и это тоже расплывчато - так зачем же рефакторинг? Как вы думаете, какие проблемы могут возникнуть в будущем, чего я не предвидел?

Обновление : я забыл упомянуть - это структура класса, которую автор предоставил в качестве решения - шаблон стратегии :

Шаблон стратегии

Адитья М.П.
источник
29
Не изучайте объектно-ориентированное программирование из книги PHP.
Георгиосирони
4
@giorgiosironi Я бросил оценивать языки. Я использую PHP каждый день, поэтому я быстрее всего изучаю новые концепции в нем. Всегда есть люди, которые ненавидят язык (Java: slidesha.re/91C4pX ) - правда, найти ненавистников Java труднее, чем диссеров PHP, но все же. Могут быть проблемы с ООП PHP, но на данный момент я не могу уделить время изучению синтаксиса Java, «пути Java» и ООП. Кроме того, настольное программирование мне чуждо. Я полный человек сети. Но прежде чем вы сможете добраться до JSP, вам, как утверждается, нужно знать настольную Java (нет цитат, я не прав?)
член парламента от Aditya,
Используйте константы для «фиксированной ставки» / «почасовой ставки» и «семинара» / «лекции» вместо строк.
Том Мартинал,

Ответы:

19

Забавно, что в книге не говорится об этом четко, но причина, по которой она предпочитает иерархию классов, если операторы внутри одного класса, вероятно, является принципом Open / Closed. Это широко известное правило разработки программного обеспечения гласит, что класс должен быть закрыт для модификация, но открытая для расширения.

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

Вы можете поместить это правило в раздел «Условные заявления», если хотите, однако я считаю этот «указатель» немного расплывчатым. Если заявления, как правило, просто запах кода, симптомы. Они могут возникнуть в результате широкого спектра плохих дизайнерских решений.

guillaume31
источник
3
Было бы гораздо лучше использовать функциональное расширение, а не наследование.
DeadMG
6
Есть, конечно, другие / лучшие подходы к проблеме. Тем не менее, ясно , что это книга о объектно-ориентированном программировании и открытом / закрытом принципе просто поразил меня , как в классическом способе приближения такого рода дизайн дилеммы в ОО.
guillaume31
Как насчет лучшего способа решения проблемы ? Нет смысла учить низшее решение только потому, что оно ОО.
DeadMG
16

Я предполагаю, что целью книги является обучение таким вещам, как:

public function cost() {
    if($this->chargeType == 'fixed rate') {
        return "30";
    } else {
        return $this->duration * 5;
    }
}

Моя интерпретация книги такова, что у вас должно быть три класса:

  • AbstractLesson
  • HourlyRateLesson
  • FixedRateLesson

Затем вы должны переопределить costфункцию в обоих подклассах.

Мое личное мнение

для простых случаев, подобных этому: не надо. Странно, что ваша книга предпочитает более глубокую иерархию классов, чтобы избежать простых условных выражений. Более того, с вашей входной спецификацией Lessonдолжна быть фабрика с отправкой, которая является условным оператором. Так что с книжным решением у вас будет:

  • 3 класса вместо 1
  • 1 уровень наследования вместо 0
  • такое же количество условных выражений

Это больше сложности без каких-либо дополнительных преимуществ.

Саймон Бергот
источник
8
В этом случае , да, это слишком сложно. Но в более крупной системе вы будете добавлять больше уроков, например SemesterLesson, MasterOneWeekLessonи т. Д. В этом случае определенно стоит использовать абстрактный корневой класс или, что еще лучше, интерфейс. Но когда у вас есть только два случая, это остается на усмотрение автора.
Майкл К
5
Я согласен с вами в целом. Тем не менее, образовательные примеры часто вымышлены и чрезмерно сконструированы, чтобы проиллюстрировать это. Вы на мгновение игнорируете другие соображения, чтобы не перепутать рассматриваемый вопрос, и представите эти соображения позже.
Карл Билефельдт
+1 Хорошая тщательная разбивка. Комментарий «Это сложнее без дополнительных преимуществ» прекрасно иллюстрирует концепцию «альтернативных издержек» в программировании. Теория! = Практичность.
Эван Плейс
7

Пример в книге довольно странный. Исходя из вашего вопроса, оригинальное утверждение:

Цель состоит в том, чтобы вывести тип урока (лекция или семинар) и плату за урок в зависимости от того, является ли это почасовым уроком или уроком с фиксированной ценой.

что в контексте главы о ООП будет означать, что автор, вероятно, хочет, чтобы вы имели следующую структуру:

+ abstract Lesson
    - Lecture inherits Lesson
    - Seminar inherits Lesson

+ abstract Charge
    - HourlyCharge inherits Charge
    - FixedCharge inherits Charge

Но потом приходит информация, которую вы цитировали:

$lessons[] = new Lesson('hourly rate', 4, 'seminar');
$lessons[] = new Lesson('fixed rate', null, 'lecture');

то есть то, что называется строго типизированным кодом, и которое, честно говоря, в этом контексте, когда читатель намеревается изучить ООП, ужасно.

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

const string $HourlyRate = 'hourly rate';
const string $FixedRate = 'fixed rate';

// [...]

Charge $charge;
switch ($chargeType)
{
    case $HourlyRate:
        $charge = new HourlyCharge();
        break;

    case $FixedRate:
        $charge = new FixedCharge();
        break;

    default:
        throw new ParserException('The value of chargeType is unexpected.');
}

// Imagine the similar code for lecture/seminar, and the similar code for both on output.

Что касается «указателей»:

  • Дублирование кода

    Ваш код не имеет дублирования. Код, который автор ожидает от вас, делает.

  • Класс, который слишком много знал о его контексте

    Ваш класс просто передает строки из входных данных в выходные и вообще ничего не знает. С другой стороны, ожидаемый код может быть подвергнут критике за то, что он знает слишком много.

  • Мастер на все руки - Классы, которые пытаются делать много вещей

    Опять же, вы просто пропускаете строки, не более того.

  • Условные заявления

    В вашем коде есть одно условное утверждение. Это читабельно и легко понять.


Возможно, на самом деле, автор ожидал, что вы напишите гораздо более переработанный код, чем тот, который содержит шесть классов выше. Например, вы можете использовать фабричный шаблон для уроков и сборов и т. Д.

+ abstract Lesson
    - Lecture inherits Lesson
    - Seminar inherits Lesson

+ abstract Charge
    - HourlyCharge inherits Charge
    - FixedCharge inherits Charge

+ LessonFactory

+ ChargeFactory

+ Parser // Uses factories to transform the input into `Lesson`.

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

Вместо этого вы получили простой для понимания, чистый код, который в десять раз короче.

Арсений Мурзенко
источник
Вау, ваши догадки точны! Посмотрите на изображение, которое я загрузил - автор рекомендовал точно такие же вещи.
депутат Адитья,
Мне бы очень хотелось принять этот ответ, но мне также нравится ответ @ ian31 :( О, что мне делать!
член парламента Адитья
5

Прежде всего вы можете изменить «магические числа», такие как 30 и 5 в функции cost (), на более значимые переменные :)

И, честно говоря, я думаю, что вы не должны беспокоиться об этом. Когда вам нужно будет изменить класс, тогда вы измените его. Попробуйте сначала создать ценность, а затем перейти к рефакторизации.

И почему ты думаешь, что ты не прав? Разве этот класс не достаточно хорош для вас? Почему вы беспокоитесь о какой-то книге;)

Михал Франк
источник
1
Спасибо за ответ! Действительно, рефакторинг придет позже. Тем не менее, я написал этот код для обучения, пытаясь решить проблему, представленную в книге, по-своему и видя, как я ошибаюсь ...
Член парламента от Адитья,
2
Но вы узнаете это позже. Когда этот класс будет использоваться в других частях системы, вам придется добавить что-то новое. Не существует решения «серебряной пули» :) Что-то может быть хорошим или плохим, это зависит от системы, требований и т. Д. При изучении ООП вам приходится много терпеть неудачу: P Затем со временем вы заметите некоторые общие проблемы и закономерности. Этот пример слишком прост, чтобы дать совет. О, я действительно рекомендую это сообщение от Джоэла :). Астронавты архитектуры берут на себя joelonsoftware.com/items/2008/05/01.html
Михал Франк
@adityamenon Тогда ваш ответ «рефакторинг придет позже». Добавляйте другие классы только тогда, когда они необходимы, чтобы сделать код проще - обычно это когда появляется третий подобный вариант использования. Смотрите ответ Саймона.
Изката
3

Совершенно не нужно реализовывать какие-либо дополнительные классы. У вас есть небольшая MAGIC_NUMBERпроблема в вашей cost()функции, но это все. Все остальное - это огромная чрезмерная инженерия. Однако, к сожалению, очень часто дается очень плохой совет - например, Circle наследует Shape. Определенно неэффективно выводить класс для простого наследования одной функции. Вы могли бы использовать функциональный подход, чтобы настроить его вместо этого.

DeadMG
источник
1
Это интересно. Можете ли вы объяснить, как бы вы сделали это на примере?
guillaume31
+1, согласен, нет причин чрезмерно проектировать / усложнять такой небольшой функционал.
GrandmasterB
@ ian31: что конкретно делать?
DeadMG
@DeadMG «использовать функциональный подход», «использовать функциональное расширение, а не наследование»
guillaume31