Принцип единой ответственности:все ли общедоступные методы в классе должны использовать все зависимости класса?

StackOverflow https://stackoverflow.com/questions/1118463

Вопрос

Скажем, у меня есть класс, который выглядит следующим образом:

internal class SomeClass
{
    IDependency _someDependency;

    ...


    internal string SomeFunctionality_MakesUseofIDependency()
    {
    ...
    }
}

И затем я хочу добавить функциональность, которая связана, но использует другую зависимость для достижения своей цели.Возможно, что-то вроде следующего:

internal class SomeClass
{
    IDependency _someDependency;

    IDependency2 _someDependency2;

    ...


    internal string SomeFunctionality_MakesUseofIDependency()
    {
    ...
    }

    internal string OtherFunctionality_MakesUseOfIDependency2()
    {
    ...
    }
}

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

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

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

Это было полезно?

Решение

Поддерживает ли SomeClass внутреннее состояние или просто «собирает» различные части функциональности?Можете ли вы переписать это так:

internal class SomeClass
{
    ...


    internal string SomeFunctionality(IDependency _someDependency)
    {
    ...
    }

    internal string OtherFunctionality(IDependency2 _someDependency2)
    {
    ...
    }
}

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

И у вас есть дополнительная ценность, заключающаяся в том, что вы можете выбрать зависимость для использования на клиенте, а не во время создания/DI.Возможно, некоторые тесты, определяющие варианты использования этих методов, помогут прояснить ситуацию:Если вы можете написать осмысленный тестовый пример, в котором оба метода вызываются для одного и того же объекта, вы не нарушите SRP.

Что касается шаблона «Фасад», то я слишком много раз видел, как он сходил с ума, чтобы он мне нравился, знаете ли, когда у вас в итоге получается класс из более чем 50 методов...Вопрос в том:Зачем тебе это?Из соображений эффективности, как в старом EJB?

Другие советы

Когда каждый метод экземпляра касается каждой переменной экземпляра, класс становится максимально сплоченным.Когда ни один из методов экземпляра не использует общую переменную экземпляра с каким-либо другим, класс является минимально связным.Хотя это правда, что нам нравится высокая сплоченность, верно и то, что применяется правило 80-20.Получение этого последнего небольшого увеличения сплоченности может потребовать огромных усилий.

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

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

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

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

Вы должны найти хороший баланс, который подойдет вам (и остальной части вашей команды).

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

ХТХ

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

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

Проблема будет в том, будет ли этот класс продолжать расти и в каком направлении.Два метода не будут иметь никакого значения, но если это повторится с более чем 3, вам нужно будет решить, хотите ли вы объявить его как фасад/адаптер или вам нужно создать дочерние классы для вариантов.

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

Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top