Принцип единой ответственности - как я могу избежать фрагментации кода?

57

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

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

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

У нас есть конструкторы классов, которые принимают более 20 параметров интерфейса, поэтому наша регистрация и разрешение IoC сами по себе становятся монстрами.

Я хочу знать, есть ли какой-либо подход «рефакторинг от SRP», который мы могли бы использовать, чтобы помочь решить некоторые из этих проблем. Я читал, что это не нарушает SOLID, если я создаю несколько пустых классов общего назначения, которые «обертывают» ряд тесно связанных классов, чтобы обеспечить единую точку доступа к сумме их функциональных возможностей (т.е. имитировать менее реализация класса чрезмерно SRP).

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

Какие-либо предложения ?

Дин Мел
источник
18
Это только мое мнение, но я думаю, что есть еще одно правило, которое очень легко забыть под кучей различных сокращений - «Принцип здравого смысла». Когда «решение» создает больше проблем, которые оно действительно решает, тогда что-то не так. Я считаю, что если проблема сложная, но заключена в класс, который заботится о ее тонкостях и все еще относительно прост в отладке - я оставляю это в покое. Вообще ваша идея «обертки» кажется мне обоснованной, но я оставлю ответ кому-то более знающему.
Патрик Свик
6
Что касается «причины для изменения» - нет необходимости преждевременно спекулировать по всем причинам. Дождитесь, когда вам действительно придется это изменить, а затем посмотрите, что можно сделать, чтобы облегчить подобные изменения в будущем.
62
Класс с 20 параметрами конструктора не кажется мне слишком SRP!
MattDavey
1
Вы пишете "... IoC регистрация и разрешение ..."; это звучит так, как будто вы (или руководитель вашей команды) думаете, что «IoC» и «внедрение зависимостей» (DI) - это одно и то же, что не соответствует действительности. DI является средством достижения IoC, но, конечно, не единственным. Вы должны тщательно проанализировать, почему вы хотите сделать IoC; если это потому, что вы хотите написать модульные тесты, то вы также можете попробовать использовать шаблон локатора службы или просто интерфейс классов ( ISomething). ИМХО, с этими подходами гораздо легче справиться, чем с внедрением зависимостей, и они дают более читаемый код
2
любой ответ, данный здесь, будет в вакууме; нам нужно увидеть код, чтобы дать конкретный ответ. 20 параметров в конструкторе? ну, вы можете пропустить объект ... или все они могут быть действительными; или они могут принадлежать в файле конфигурации, или они могут принадлежать к классу DI, или ... Симптомы, безусловно, звучат подозрительно, но, как и большинство вещей в CS, «это зависит» ...
Стивен А. Лоу

Ответы:

85

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

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

Итак, я думаю, что я говорю ... SRP не ваша проблема. Это либо неправильное понимание SRP, либо невероятно детальное его применение. Постарайся заставить свою команду держать главное главное. И главное - ремонтопригодность.

РЕДАКТИРОВАТЬ

Заставьте людей разрабатывать модули таким образом, чтобы это облегчало их использование. Думайте о каждом классе как о мини-API. Сначала подумайте: «Как бы я хотел использовать этот класс», а затем реализовать его. Не просто думайте: «Что должен делать этот класс?» SRP действительно имеет тенденцию усложнять использование классов, если вы не уделяете много внимания удобству использования.

РЕДАКТИРОВАТЬ 2

Если вы ищете советы по рефакторингу, вы можете начать делать то, что вы предлагали - создавать более грубые классы, чтобы обернуть несколько других. Убедитесь, что более грубый класс все еще придерживается SRP , но на более высоком уровне. Тогда у вас есть две альтернативы:

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

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

Фил
источник
2
Альтернативный способ заставить людей задуматься о разработке классов: пусть они пишут CRC-карты (Class Name, Responsibility, Collaborators) . Если в классе слишком много соавторов или обязанностей, скорее всего, недостаточно SRP. Другими словами, весь текст должен уместиться в карточке-указателе, иначе он делает слишком много.
Спойк
18
Я знаю, для чего нужен огнемет, но как, черт возьми, ты ловишь рыбу на шесте?
Р. Мартиньо Фернандес
13
+1 ТВЕРДЫЙ - это средство для достижения цели, а не самоцель.
B Семь
1
+1: раньше я утверждал, что такие вещи, как «Закон Деметры», названы неправильно, это должна быть «Линия гида Деметры». Эти вещи должны работать на вас, вы не должны работать на них.
Binary Worrier
2
@EmmadKareem: Это правда, что объекты DAO должны иметь несколько свойств. Но опять же, есть несколько вещей, которые вы можете сгруппировать во что-то такое простое, как Customerкласс, и иметь более понятный код. Смотрите примеры здесь: codemonkeyism.com/...
Spoike
33

Я думаю, что в Рефакторинге Мартина Фаулера я однажды прочитал контр-правило для SRP, определяя, куда оно заходит слишком далеко. Есть второй вопрос, такой же важный, как «у каждого класса есть только одна причина для изменения?» и то, что "каждое изменение затрагивает только один класс?"

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

Например, если добавление одного поля в таблицу означает, что вы должны изменить DTO и класс валидатора, а также класс персистентности и объект модели представления и т. Д., То вы создали проблему. Возможно, вам следует переосмыслить, как вы реализовали SRP.

Возможно, вы сказали, что добавление поля является причиной изменения объекта Customer, но изменение уровня постоянства (скажем, из файла XML в базу данных) - еще одна причина для изменения объекта Customer. Таким образом, вы решили создать объект CustomerPersistence. Но если вы делаете это так, что добавление поля STILL требует изменения объекта CustomerPersisitence, тогда какой смысл? У вас все еще есть объект с двумя причинами для изменения - он просто больше не Клиент.

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

Таким образом, вот что я склонен делать: если есть грубый баланс между количеством раз, когда я говорю «нет, есть более чем одна причина изменить этот объект» и количеством раз, когда я говорю «нет, это изменение будет затрагивать более одного объекта ", тогда я думаю, что у меня есть правильный баланс между SRP и фрагментацией. Но если оба они все еще высоки, я начинаю задумываться, а можно ли по-другому разделить проблемы?

прецизионный самописец
источник
+1 за "каждое изменение влияет только на один класс?"
dj18
Связанная с этим проблема, о которой я не говорил, заключается в том, что если задачи, связанные с одним логическим объектом, фрагментируются между различными классами, то может потребоваться, чтобы код содержал ссылки на несколько различных объектов, которые все связаны с одним и тем же объектом. Рассмотрим, например, печь с функциями «SetHeaterOutput» и «MeasureTe Temperature». Если печь была представлена ​​независимыми объектами HeaterControl и TemperatureSensor, то ничто не помешало бы объекту TemperatureFeedbackSystem содержать ссылку на нагреватель одной печи и другой датчик температуры печи.
суперкат
1
Если бы вместо этого эти функции были объединены в интерфейс IKiln, который был реализован объектом Kiln, то TemperatureFeedbackSystem должна была бы содержать только одну ссылку IKiln. Если бы было необходимо использовать печь с независимым датчиком температуры вторичного рынка, можно было бы использовать объект CompositeKiln, конструктор которого принял IHeaterControl и ITemperaSensor и использовал их для реализации IKiln, но такая преднамеренная неплотная композиция легко распознавалась бы в коде.
суперкат
24

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

public class MyAwesomeClass {
    public class MyAwesomeClass(IDependency1 _d1, IDependency2 _d2, ... , IDependency20 _d20) {
      // Assign it all
    }
}

... тогда это стало слишком сложно, и ты не следишь за SRP , не так ли? Могу поспорить, если бы вы записали, что MyAwesomeClassна карточке CRC она не помещается на карточку, или вы должны писать очень мелкими неразборчивыми буквами.

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

TDD даст вам хороший показатель того, сколько делает класс. Прямо поставил; если у тестового метода есть код установки, на запись которого уходит вечность (даже если вы реорганизуете тесты), то у вас, MyAwesomeClassвероятно, слишком много дел.

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

  1. Определите все действия (или обязанности), которые ваш класс выполняет со своими зависимостями.
  2. Сгруппируйте действия в соответствии с тесно связанными зависимостями.
  3. Дать полномочия! Т.е. рефакторинг каждого из выявленных действий либо новым, либо (что более важно) другим классам.

Абстрактный пример рефакторинга ответственности

Пусть Cбудет класс , который имеет несколько зависимостей D1, D2, D3, D4что вам нужно реорганизовать , чтобы использовать меньше. Когда мы определяем, какие методы Cвызывают зависимости, мы можем составить простой список:

  • D1- performA(D2),performB()
  • D2 - performD(D1)
  • D3 - performE()
  • D4 - performF(D3)

Глядя на список, мы видим это D1и D2связаны друг с другом, так как класс так или иначе нуждается в них. Мы также можем видеть, что D4необходимо D3. Итак, у нас есть две группировки:

  • Group 1- D1<->D2
  • Group 2- D4->D3

Группировка является показателем того, что у класса теперь есть две обязанности.

  1. Group 1- Один для обработки вызова двух объектов, которые нужны друг другу. Возможно, вы можете позволить своему классу Cустранить необходимость обработки обеих зависимостей и оставить одну из них для обработки этих вызовов. В этой группе очевидно, что D1может иметь ссылку на D2.
  2. Group 2- Другая ответственность нуждается в одном объекте, чтобы вызвать другой. Не можете D4справиться D3вместо вашего класса? Тогда мы, вероятно, можем исключить D3из класса C, разрешив D4вместо этого делать вызовы.

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


Редактировать:

Среди комментариев @ Эммад Карем говорит:

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

Это правда, что объекты DAO имеют тенденцию иметь много параметров, которые вы должны установить в своем конструкторе, и параметры, как правило, являются простыми типами, такими как string. Однако в примере с Customerклассом вы можете сгруппировать его свойства внутри других классов, чтобы упростить задачу. Например, наличие Addressкласса с улицами и Zipcodeкласса, который содержит почтовый индекс и будет обрабатывать бизнес-логику, такую ​​как проверка данных:

public class Address {
    private String street1;
    //...

    private Zipcode zipcode;

    // easy to extend
    public bool isValid() {
        return zipcode.isValid();
    }
}

public class Zipcode {
    private string zipcode;
    public bool isValid() {
        // return regex match that zipcode contains numbers
    }
}

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

Spoike
источник
+1: отличный ответ! Группировка является IMO очень мощным механизмом, потому что вы можете применять группирование рекурсивно. Говоря очень грубо, с n уровнями абстракции вы можете организовать 2 ^ n элементов.
Джорджио
+1: Ваши первые несколько абзацев точно отражают то, с чем сталкивается моя команда. «Бизнес-объекты», которые на самом деле являются объектами обслуживания, и код установки модульного теста, который просто ошеломляет писать. Я знал, что у нас есть проблема, когда наши вызовы уровня обслуживания будут содержать одну строку кода; вызов метода бизнес-уровня.
Мужчина
3

Я согласен со всеми ответами о SRP и о том, как можно зайти слишком далеко. В своем посте вы упоминаете, что из-за «чрезмерного рефакторинга» для соблюдения SRP вы обнаружили нарушение инкапсуляции или изменение. Единственное, что сработало для меня, - это всегда придерживаться основ и делать именно то, что требуется для достижения цели.

При работе с системами Legacy «энтузиазм» исправления всего, что делает его лучше, обычно достаточно велик в Team Leads, особенно тех, кто новичок в этой роли. SOLID, просто нет SRP - это просто S. Убедитесь, что если вы следуете SOLID, вы также не забудете OLID.

Я сейчас работаю над системой Legacy, и в начале мы начали идти по тому же пути. Для нас сработало коллективное решение команды сделать лучшее из обоих миров - SOLID и KISS (Keep It Simple Stupid). Мы коллективно обсудили основные изменения в структуре кода и применили здравый смысл при применении различных принципов разработки. Они великолепны в качестве руководства, а не «Законов развития ПО». Команда - это не только командный лидер, а все разработчики в команде. То, что всегда работало для меня, - это собрать всех в комнате и выработать общий набор правил, которым вся команда соглашается следовать.

Что касается того, как исправить текущую ситуацию, если вы используете VCS и не добавили слишком много новых функций в свое приложение, вы всегда можете вернуться к версии кода, которую вся команда считает понятной, читаемой и поддерживаемой. Да! Я прошу вас отказаться от работы и начать с нуля. Это лучше, чем пытаться «починить» что-то, что было сломано, и вернуть его к тому, что уже существовало.

Шарат Сатиш
источник
3

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

Всякий раз, когда я оцениваю реструктуризацию кода или добавляю новую функцию, я думаю о том, сколько шаблонов потребуется по сравнению с реальной логикой. Если ответ превышает 50%, это, вероятно, означает, что я слишком обдумываю это.

Помимо SRP, есть много других стилей разработки. В вашем случае это звучит как YAGNI, безусловно, не хватает.

cmcginty
источник
3

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

Вы можете увидеть блог Боба здесь об этой ситуации, но он утверждает , что если ответственность размазывается по нескольким классам , то ответственность SRP нарушается , поскольку эти классы меняются параллельно. Я подозреваю, что вашему разработчику действительно понравится дизайн в верхней части блога Боба, и он может быть немного разочарован, увидев его разорванным на части. В частности, потому что это нарушает «Общий принцип закрытия» - вещи, которые меняются вместе, остаются вместе.

Помните, что SRP относится к «причине изменения», а не «делать что-то одно», и что вам не нужно заботиться о причине изменения до тех пор, пока изменение не произойдет. Второй парень платит за абстракцию.

Теперь есть вторая проблема - «яростный сторонник разработки SOLID». Конечно, не похоже, что у вас хорошие отношения с этим разработчиком, поэтому любые попытки убедить его / ее в проблемах в кодовой базе поставлены в тупик. Вам нужно будет восстановить отношения, чтобы вы могли по-настоящему обсудить проблемы. Я бы порекомендовал это пиво.

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

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

Эрик Смит
источник
-1

Я согласен с вашим решением [update = 2012.05.31] о том, что SRP обычно является хорошей идеей. Но я полностью согласен с комментарием @ Spoike -s о том, что конструктор с 20 аргументами интерфейса - это очень много. [/ Update]:

Введение SRP с IoC перемещает сложность от одного «класса с множественной ответственностью» ко многим классам srp и намного более сложной инициализации в пользу

  • упрощенная модульная тестируемость / tdd (тестирование одного srp-класса за раз)
  • но за счет
    • гораздо более сложная инициализация кода и интеграция и
    • более сложная отладка
    • фрагментация (= распределение кода по нескольким файлам / каталогам)

Боюсь, вы не можете уменьшить фрагментацию кода, не жертвуя srp.

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

   class MySrpClass {
      MySrpClass(Interface1 parm1, Interface2 param2, .... Interface20 param2) {
      }
   } 

   class MySyntaxSugarClass : MySrpClass {
      MySyntaxSugarClass() {
         super(new MyInterface1Implementation(), new MyImpl2(), ....)
      }
   }
k3b
источник
2
Я считаю, что 20 интерфейсов - это показатель того, что у класса слишком много дел. То есть есть 20 причин для его изменения, что в значительной степени является нарушением SRP. То, что система сложна, не означает, что она должна быть сложной.
Спойк