Вопрос о примере в «Чистом коде» Роберта С. Мартина

Question

Это вопрос о концепции функции, выполняющей только одно.Это не будет иметь смысла без некоторых соответствующих отрывков для контекста, поэтому я приведу их здесь.Они появляются на стр. 37-38:

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

Чтобы включить настройки и разрывы, мы включаем настройки, затем включаем содержимое тестовой страницы и затем включаем разрывы.Чтобы включить настройки, мы включаем настройку набора, если это набор, затем включаем обычную настройку.

Оказывается, что программистам очень трудно научиться следовать этому правилу и писать функциикоторые остаются на одном уровне абстракции.Но изучение этого трюка также очень важно.Это ключ к тому, чтобы функции были короткими и чтобы они выполняли «одну вещь». Создание кода, читаемого как нисходящий набор абзацев TO, является эффективным способом поддержания согласованности уровня абстракции.

Затем он приводит следующий пример плохого кода:

public Money calculatePay(Employee e) 
throws InvalidEmployeeType {
switch (e.type) {
  case COMMISSIONED:
    return calculateCommissionedPay(e);
  case HOURLY:
    return calculateHourlyPay(e);
  case SALARIED:
    return calculateSalariedPay(e);
  default:
    throw new InvalidEmployeeType(e.type);
}
}

и объясняет проблемы с ним следующим образом:

Есть несколько проблем с этой функцией.Во-первых, он большой, и когда добавляются новые типы сотрудников, он будет расти.Во-вторых, это очень четко делает больше, чем одно.В-третьих, он нарушает принцип единой ответственности7 (SRP), поскольку существует более чем одна причина для его изменения.В-четвертых, он нарушает принцип Open Closed Principle8 (OCP), поскольку он должен меняться при каждом добавлении новых типов.

Теперь мои вопросы.

Для начала, мне ясно, как он нарушаетOCP, и мне ясно, что только это делает его плохим дизайном.Однако я пытаюсь понять каждый принцип, и мне не ясно, как применяется SRP.В частности, единственная причина, по которой я могу представить изменение этого метода, - это добавление новых типов сотрудников.Существует только одна «ось изменения».Если детали расчета необходимо изменить, это повлияет только на такие подметоды, как "CalcualHourlyPay ()"

Кроме того, хотя, в некотором смысле, очевидно, что он делает 3 вещи, все эти три вещи находятся на одном уровнеабстракция, и все они могут быть помещены в абзац TO, не отличающийся от примера: Чтобы рассчитать оплату за сотрудника, мы рассчитываем комиссионную плату, если сотрудник получает комиссию, почасовую оплату, если он является почасовым, и т. д. .Таким образом, помимо нарушения OCP, этот код, похоже, соответствует другим требованиям Мартина к чистому коду, хотя он утверждает, что это не так.

Может кто-нибудь объяснить, что мне не хватает?

Спасибо.

Answer 1

По-видимому, есть две причины для изменения calcPay:

1. Изменения в типах сотрудников
2. Изменения в расчетах заработной платы

Две разные осименять.Однако ответственность за метод CalculatePay лежит расчет оплаты.Он должен изменяться только в случае изменения формулы расчета заработной платы.Я думаю, именно поэтому автор заявляет, что метод нарушает ПСП.

В книге автор предлагает решение, в котором он определяет классы для каждого типа сотрудника, полученные из общего абстрактного базового класса Employee.Он перемещает метод calcPay в базовый класс Employee и определяет класс фабрики Employee, который создает соответствующие объекты сотрудника с учетом типа сотрудника.Таким образом, каждый расчет заработной платы инкапсулируется в определенный класс типов сотрудников и поэтому не подвержен изменениям в типах сотрудников.Кроме того, на класс фабрики сотрудников в этом простом решении влияют только изменения типов сотрудников.Таким образом, новое решение делает SRP счастливым.

В новом решении вы просите сотрудника рассчитать свою зарплату, которая мне не нравится, потому что это не отражает реальность.Вы можете утверждать, что это тоже является нарушением ПСП.Этот расчет является обязанностью отдела начисления заработной платы.Мне нравится, когда модель в программном обеспечении максимально отражает область реального мира, но нам обычно приходится идти на компромиссы.В этом случае я бы сказал, что изменения в типах сотрудников не будут происходить на регулярной основе.На самом деле, они, скорее всего, будут происходить очень редко.Таким образом, я хотел бы сохранить вещи там, где они имеют смысл для бизнес-сферы, такие как запрос объекта заработной платы для расчета заработной платы сотрудника.В то же время я хотел бы иметь и хранить обширный набор юнит-тестов, так как нужно следить за тем, чтобы при изменении типа сотрудника все продолжало работать так, как ожидалось.

Answer 2

Я также пытался понять это, и я думаю, что мог бы найти что-то, чтобы убедить вас, если нет.

На самом деле есть две причины изменить этот метод:

1. расчет оплаты: поскольку по любой причине вы можете решить, что теперь HourlyPaidEmployee будет оплачиваться с использованием ежемесячной стратегии. Вы изменяете не правильную формулу расчета, а то, как вы платите этому типу работника, что по сути является изменением на том же уровне абстракции.
2. изменение типов сотрудников: вы можете добавить другого типа сотрудника (фрилансера) или даже переименовать тип в другое.

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

Answer 3

Я делаю длинный выстрел, потому что мне не хватает контекста.Этот метод может измениться из-за ДВУХ причин (кстати, код также нарушает основной принцип инкапсуляции)

1. Когда добавляется новый тип сотрудника, но стратегия расчета зарплаты соответствует существующим стратегиям
2. Когда добавляется новый тип сотрудника И, и для этого требуется новая стратегия расчета заработной платы

В обоих случаях абстракция, которую необходимо изменить, - это добавляемый новый тип сотрудника, а не клиент /пользователь класса Employee.Я имею в виду, что (метод расчета зарплаты должен быть инкапсулирован) метод convertPay () принадлежит абстракции Employee, что-то вроде ниже

interface SalariedEmployee
{
BigDecimal calculatePay();
}

class HourlyEmployee implements SalariedEmployee
{
}

class CommissionedEmployee implements SalariedEmployee
{
}