Уменьшение сложности класса

10

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

Моя абстрактная проблема в том, что у меня есть объект и мне нужно выполнить длинную последовательность операций над ним; Я думаю об этом как о сборочной линии, как о сборке машины.

Я считаю, что эти объекты будут называться объектами метода .

Так что в этом примере в какой-то момент у меня будет CarWithoutUpholstery, на котором мне нужно будет запустить installBackSeat, installFrontSeat, installWoodenInserts (операции не мешают друг другу и могут даже выполняться параллельно). Эти операции выполняются CarWithoutUpholstery.worker () и дают новый объект, который будет CarWithUpholstery, для которого я затем запустил бы, возможно, cleanInsides (), verifyNoUpholsteryDefects () и так далее.

Операции в одной фазе уже независимы, т. Е. Я уже имею дело с их подмножеством, которое может быть выполнено в любом порядке (переднее и заднее сидения могут быть установлены в любом порядке).

Моя логика в настоящее время использует Reflection для простоты реализации.

То есть, когда у меня есть CarWithoutUpholstery, объект проверяет себя на наличие методов с именем executeSomething (). В этот момент он выполняет все эти методы:

myObject.perform001SomeOperation();
myObject.perform002SomeOtherOperation();
...

при проверке ошибок и прочего. Хотя порядок операций не важен, я назначил лексикографический порядок на случай, если когда-нибудь обнаружу, что какой-то порядок важен в конце концов. Это противоречит YAGNI , но стоит очень мало - простая сортировка () - и это может сэкономить массовое переименование метода (или введение какого-либо другого метода выполнения тестов, например, массива методов).

Другой пример

Скажем, вместо того, чтобы строить машину, я должен составить отчет Секретной полиции о ком-то и передать его моему Злому Повелителю . Мой последний объект будет ReadyReport. Чтобы построить его, я начинаю со сбора основной информации (имя, фамилия, супруг ...). Это моя фаза A. В зависимости от того, есть ли супруг или нет, мне, возможно, придется перейти к фазам B1 или B2 и собрать данные о сексуальности для одного или двух человек. Это сделано из нескольких разных запросов к различным Злым Миньонам, контролирующим ночную жизнь, уличные камеры, квитанции о продажах в секс-шопах и тому подобное. И так далее.

Если у жертвы нет семьи, я даже не войду в фазу GetInformationAboutFamily, но если я это сделаю, то не имеет значения, нацелена ли я сначала на отца, на мать или на братьев и сестер (если есть). Но я не могу этого сделать, если я не выполнил FamilyStatusCheck, который, следовательно, относится к более раннему этапу.

Все работает чудесно ...

  • если мне нужна дополнительная операция, мне нужно только добавить приватный метод,
  • если операция является общей для нескольких этапов, я могу унаследовать ее от суперкласса,
  • операции просты и автономны. Ни одно значение из одной операции не всегда требует какой - либо из других (операций , которые действительно выполняются в другой фазе),
  • Объекты, находящиеся внизу линии, не должны выполнять много тестов, поскольку они даже не могли бы существовать, если их создатели не проверили эти условия в первую очередь. Т.е. при размещении вставок в приборной панели, очистке приборной панели и проверке приборной панели мне не нужно проверять, что приборная панель действительно есть .
  • это позволяет легко тестировать. Я могу легко смоделировать частичный объект и запустить любой метод для него, и все операции являются детерминированными черными ящиками.

...но...

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

Я уже поднял этот вопрос наверх и предположил, что в этом случае множество частных методов не является признаком катастрофы. Сложность это есть, но она есть , потому что операция является сложной, и на самом деле это не все , что комплекс - это просто долго .

Используя пример Злого Повелителя, моя проблема в том, что Злой Повелитель (он же Тот, кому не будет отказано ), запросив всю диетическую информацию, мои диетические миньоны сказали мне, что мне нужно запрашивать рестораны, мини-кухни, уличных торговцев, нелицензированных уличных торговцев, теплицу владельцы и т. д., и Злой (подчиненный) Повелитель - известный также как Тот, Кто также не откажется - жалующийся на то, что я выполняю слишком много запросов на этапе GetDietaryInformation.

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

Что я думаю, я мог сделать

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

  • Я проверил, что могу быть хитрым и объявить половину моих методов protected. Но я бы использовал слабость в процедуре тестирования, и кроме оправдания себя, когда меня поймали, мне это не нравится. Кроме того, это временная мера. Что если число необходимых операций удваивается? Вряд ли, но что тогда?
  • Я могу произвольно разделить эту фазу на AnnealedObjectAlpha, AnnealedObjectBravo и AnnealedObjectCharlie, и иметь одну треть операций, выполняемых на каждом этапе. У меня сложилось впечатление, что это на самом деле добавляет сложности (N-1 больше классов), без каких-либо преимуществ, кроме прохождения теста. Конечно, я могу утверждать, что CarWithFrontSeatsInstalled и CarWithAllSeatsInstalled являются логически последовательными этапами. Риск того, что метод Браво позже понадобится Альфе, невелик и даже меньше, если я буду хорошо его играть. Но все равно.
  • Я могу объединить различные операции, удаленно похожие, в одну. performAllSeatsInstallation(), Это только временная мера, и она увеличивает сложность одной операции. Если мне когда-нибудь понадобится выполнить операции A и B в другом порядке, и я упаковал их в E = (A + C) и F (B + D), мне придется разложить E и F и перемешать код вокруг ,
  • Я могу использовать массив лямбда-функций и аккуратно обходить проверку, но я нахожу это неуклюжим. Однако это пока лучшая альтернатива. Это избавило бы от размышлений. У меня есть две проблемы: меня, вероятно, попросят переписать все объекты методов, не только гипотетические CarWithEngineInstalled, и хотя это будет очень хорошей защитой работы, на самом деле это не так уж и привлекательно; и что у проверки покрытия кода есть проблемы с лямбдами (которые разрешимы, но все же ).

Так...

  • Какой, ты думаешь, мой лучший вариант?
  • Есть ли лучший способ, который я не рассмотрел? ( возможно, мне лучше прийти и спросить, что это? )
  • Неужели этот дизайн безнадежно испорчен, и мне лучше признать поражение и рвоту - эту архитектуру вообще? Не хорошо для моей карьеры, но будет ли написание плохо спроектированного кода лучше в долгосрочной перспективе?
  • Является ли мой текущий выбор «Единственно верным путем», и мне нужно бороться, чтобы установить метрики лучшего качества (и / или контрольно-измерительные приборы)? Для этого последнего варианта мне понадобятся ссылки ... Я не могу просто махнуть рукой на @PHB, бормоча. Это не те показатели, которые вы ищете . Независимо от того, сколько я хотел бы иметь возможность
LSerni
источник
3
Или вы можете игнорировать предупреждение «превышена максимальная сложность».
Роберт Харви
Если бы я мог, я бы. Каким-то образом эта метрическая вещь приобрела священное качество само по себе. Я продолжаю пытаться убедить PTB принять их как полезный инструмент, но только как инструмент. Я еще могу добиться успеха ...
LSerni
Действительно ли операции конструируют объект или вы просто используете метафору сборочной линии, потому что она разделяет упомянутое вами конкретное свойство (многие операции с частичным порядком зависимости)? Значение в том , что бывший предлагает шаблон строитель, в то время как последний может предложить некоторые другие модели с более подробной заполненными.
outis
2
Тирания метрик. Метрики являются полезными индикаторами, но их применение, игнорируя, почему эта метрика используется, не помогает.
Джейди
2
Объясните людям наверху разницу между существенной сложностью и случайной сложностью. en.wikipedia.org/wiki/No_Silver_Bullet Если вы уверены, что сложность, которая нарушает правило, является существенной, то если вы будете проводить рефакторинг для programmers.stackexchange.com/a/297414/51948, вы, вероятно, будете кататься вокруг правила и распространять из сложности. Если инкапсуляция вещей в фазы не является произвольной (фазы связаны с проблемой) и редизайн уменьшает когнитивную нагрузку для разработчиков, поддерживающих код, то имеет смысл сделать это.
Фурманатор

Ответы:

15

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

Чтобы построить пример сборки вашего автомобиля, представьте Carкласс:

public class Car
{
    public IChassis Chassis { get; private set; }
    public Dashboard { get; private set; }
    public IList<Seat> Seats { get; private set; }

    public Car()
    {
        Seats = new List<Seat>();
    }

    public void AddChassis(IChassis chassis)
    {
        Chassis = chassis;
    }

    public void AddDashboard(Dashboard dashboard)
    {
        Dashboard = dashboard;
    }
}

Я собираюсь опустить реализации некоторых компонентов, например IChassis, Seatи Dashboardради краткости.

CarНе несет ответственности за само здание. Сборочная линия есть, так что давайте инкапсулируем это в класс:

public class CarAssembler
{
    protected List<IAssemblyPhase> Phases { get; private set; }

    public CarAssembler()
    {
        Phases = new List<IAssemblyPhase>()
        {
            new ChassisAssemblyPhase(),
            new DashboardAssemblyPhase(),
            new SeatAssemblyPhase()
        };
    }

    public void Assemble(Car car)
    {
        foreach (IAssemblyPhase phase in Phases)
        {
            phase.Assemble(car);
        }
    }
}

Важным отличием здесь является то, что CarAssembler имеет список реализуемых объектов фазы сборки IAssemblyPhase. Теперь вы имеете дело с общедоступными методами, то есть каждая фаза может быть протестирована сама по себе, и ваш инструмент качества кода более удачен, потому что вы не вкладываете так много в один класс. Процесс сборки тоже очень прост. Просто переключайтесь между фазами и вызывайте Assembleпроезжающие машины. Выполнено. IAssemblyPhaseИнтерфейс также мертв просто с помощью всего одного общедоступного метода , который принимает объект автомобиля:

public interface IAssemblyPhase
{
    void Assemble(Car car);
}

Теперь нам нужны наши конкретные классы, реализующие этот интерфейс для каждой конкретной фазы:

public class ChassisAssemblyPhase : IAssemblyPhase
{
    public void Assemble(Car car)
    {
        car.AddChassis(new UnibodyChassis());
    }
}

public class DashboardAssemblyPhase : IAssemblyPhase
{
    public void Assemble(Car car)
    {
        Dashboard dashboard = new Dashboard();

        dashboard.AddComponent(new Speedometer());
        dashboard.AddComponent(new FuelGuage());
        dashboard.Trim = DashboardTrimType.Aluminum;

        car.AddDashboard(dashboard);
    }
}

public class SeatAssemblyPhase : IAssemblyPhase
{
    public void Assemble(Car car)
    {
        car.Seats.Add(new Seat());
        car.Seats.Add(new Seat());
        car.Seats.Add(new Seat());
        car.Seats.Add(new Seat());
    }
}

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

Даже если вы сказали, что порядок не имеет значения сейчас, это может произойти в будущем.

Чтобы закончить это, давайте посмотрим на процесс сборки автомобиля:

Car car = new Car();
CarAssembler assemblyLine = new CarAssembler();

assemblyLine.Assemble(car);

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


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

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

Разработка программного обеспечения - это вечный процесс обучения, применения, а затем повторного обучения. То, что вы решили переписать свой код, не означает, что вас уволят. Если бы это было так, то не было бы разработчиков программного обеспечения.

Является ли мой текущий выбор «Единственно верным путем», и мне нужно бороться, чтобы установить метрики лучшего качества (и / или контрольно-измерительные приборы)?

То, что вы обрисовали в общих чертах, не является Единственно верным путем, однако имейте в виду, что метрики кода также не являются Единственно верным путем. Метрики кода следует рассматривать как предупреждения. Они могут указывать на проблемы с техническим обслуживанием в будущем. Это руководящие принципы, а не законы. Когда ваш инструмент метрик кода что-то указывает, я сначала исследую код, чтобы увидеть, правильно ли он реализует принципы SOLID . Многократный рефакторинг кода, чтобы сделать его более твердым, удовлетворит метрики кода. Иногда это не так. Вы должны принять эти показатели в каждом конкретном случае.

Грег Бургардт
источник
1
Да, намного лучше, чем один класс бога.
BЈовић
Этот подход работает, но в основном потому, что вы можете получить элементы для автомобиля без параметров. Что если вам нужно их передать? Затем вы можете выбросить все это из окна и полностью переосмыслить процедуру, потому что на самом деле у вас нет автомобильного завода со 150 параметрами, это безумие.
Энди
@DavidPacker: Даже если вам нужно параметризовать кучу вещей, вы можете инкапсулировать это также в некоторый класс Config, который вы передаете в конструктор вашего объекта «ассемблера». В этом примере автомобиля цвет краски, цвет салона и материалы, комплектация, двигатель и многое другое можно настроить, но у вас не обязательно будет 150 настроек. У вас, вероятно, будет дюжина или около того, которая поддается объекту опций.
Грег Бургхардт
Но только добавление конфигурации может бросить вызов назначению красивого цикла for, что является огромным позором, потому что вам придется анализировать конфигурацию и назначать ее подходящим методам, которые в свою очередь будут давать вам надлежащие объекты. Таким образом, вы, вероятно, в итоге получили бы что-то очень похожее на оригинальный дизайн.
Энди
Понимаю. Вместо одного класса и пятидесяти методов у меня было бы пятьдесят классов для ассемблера и один оркестрирующий класс. В конечном итоге результат выглядит одинаково, также с точки зрения производительности, но компоновка кода становится чище. Мне это нравится. Операции добавления будут немного более неловкими (мне придется настроить их в своем классе, а также проинформировать оркеструющий класс; я не вижу простого выхода из этой необходимости), но все же мне это очень нравится. Позвольте мне пару дней изучить этот подход.
LSerni
1

Я не знаю, возможно ли это для вас, но я подумал бы об использовании более ориентированного на данные подхода. Идея состоит в том, что вы фиксируете некоторое (декларативное) выражение правил и ограничений, операций, зависимостей, состояния и т. Д. Тогда ваши классы и код более универсальны, ориентированы на соблюдение правил, инициализацию текущего состояния экземпляров, выполнение операций для изменить состояние, используя декларативный захват правил и изменений состояния, а не используя код более напрямую. Можно использовать объявления данных на языке программирования, или некоторые инструменты DSL, или правила или логический движок.

Эрик Эйдт
источник
Некоторые операции фактически реализованы таким образом (в виде списков правил). Для примера с автомобилем, при установке коробки предохранителей, лампочек и штекеров, я на самом деле не использую три разных метода, а только один, который в общем подходит для двухштырьковых электрических штуцеров, и принимает список из трех списков предохранителей. , огни и вилки. Подавляющее большинство других методов, к сожалению, не поддаются такому подходу.
LSerni
1

Почему-то эта проблема напоминает мне зависимости. Вы хотите автомобиль в определенной конфигурации. Как и Грег Бургхардт, я бы использовал объекты / классы для каждого шага / элемента /….

Каждый шаг объявляет, что он добавляет к миксу (что это provides) (может быть несколько вещей), но также и то, что он требует.

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

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

Наконец, сборка вашего автомобиля сводится к тому, что возможные шаги, и позволить CarBuilder выяснить необходимые шаги.

Однако важно отметить, что вы должны найти способ сообщить CarBuilder обо всех шагах. Попробуйте сделать это с помощью файла конфигурации. Для добавления дополнительного шага потребуется только дополнение к файлу конфигурации. Если вам нужна логика, попробуйте добавить DSL в файл конфигурации. Если ничего не помогает, вы застряли, определяя их в коде.

Sjoerd Job Postmus
источник
0

Несколько вещей, которые вы упоминаете, выделяются как «плохие» для меня

«Моя логика в настоящее время использует Reflection для простоты реализации»

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

«Операции в одной фазе уже независимы»

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

MyObject.AddComponent(IComponent component) 

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

На самом деле я представляю, что они не являются по-настоящему независимыми, я представляю, что каждый проверяет состояние объекта и изменяет его или, по крайней мере, выполняет какую-то проверку перед началом.

В этом случае вы можете:

  1. Выкиньте ООП из окна и имейте сервисы, которые работают с открытыми данными в вашем классе, т.е.

    Service.AddDoorToCar (Автомобиль)

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

    Car = CarBuilder.WithDoor (). WithDoor (). WithWheels ();

(логику withX можно снова разбить на подстройщики)

  1. Используйте функциональный подход и передайте функции / delgates в метод модификации

    Car.Modify ((c) => c.doors ++);

Ewan
источник
Эван сказал: «Выкинь ООП из окна и имей сервисы, которые работают с открытыми данными в твоем классе» --- Как это не объектно-ориентировано?
Грег Бургхардт
?? его нет, это не OO вариант
Ewan
Вы используете объекты, что означает, что это «объектно-ориентированный». Тот факт, что вы не можете определить шаблон программирования для своего кода, не означает, что он не является объектно-ориентированным. Твердые принципы - это хорошее правило. Если вы внедрили некоторые или все принципы SOLID, это объектно-ориентированный. Действительно, если вы используете объекты, а не просто передаете структуры различным процедурам или статическим методам, это объектно-ориентированный. Существует разница между «объектно-ориентированным кодом» и «хорошим кодом». Вы можете написать плохой объектно-ориентированный код.
Грег Бургхардт
это не OO, потому что вы представляете данные, как если бы они были структурой, и работаете с ними в отдельном классе, как в процедуре
Ewan
Я только добавил ведущий комментарий, чтобы попытаться остановить неизбежное «это не ООП!» комментарии
Ewan