Разработка класса, который будет принимать целые классы в качестве параметров, а не отдельные свойства

30

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

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

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

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

Пользователь - только один пример. Это широко практикуется в нашем коде.

Правильно ли я считаю, что это нарушение принципа Open / Closed? Не просто акт изменения существующих классов, но в первую очередь их настройка, чтобы в будущем потребовались широкомасштабные изменения?

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

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

Существуют ли другие, не твердые принципы, которые нарушаются, такие как базовые принципы защитного программирования?

Должен ли мой конструктор выглядеть так:

MyConstructor(GUID userid, String username)

Или это:

MyConstructor(User theUser)

Редактировать сообщение:

Было предложено ответить на этот вопрос в «Pass ID или Object?». Это не отвечает на вопрос о том, как решение пойти в ту или иную сторону влияет на попытку следовать принципам SOLID, что лежит в основе этого вопроса.

Джимбо
источник
11
@gnat: Это определенно не дубликат. Возможный дубликат о цепочке методов, чтобы проникнуть глубоко в иерархию объектов. Этот вопрос, кажется, не задает об этом вообще.
Грег Бургхардт
2
Вторая форма часто используется, когда количество передаваемых параметров стало громоздким.
Роберт Харви
12
Что мне не нравится в первой подписи, так это то, что нет никакой гарантии, что userId и username на самом деле происходят от одного и того же пользователя. Это потенциальная ошибка, которую можно избежать, обойдя пользователя повсюду. Но решение действительно зависит от того, что вызванные методы делают с аргументами.
17 из 26
9
Слово «разбирать» не имеет смысла в контексте, в котором вы его используете. Вы имели в виду «пройти» вместо этого?
Конрад Рудольф
5
Как насчет Iдюйма SOLID? MyConstructorв основном говорит сейчас "мне нужно Guidи string". Так почему бы не иметь интерфейс, обеспечивающий a Guidи a string, позволить Userреализовать этот интерфейс и позволить MyConstructorзависеть от экземпляра, реализующего этот интерфейс? А если нужно MyConstructorпоменять, поменяйте интерфейс. - Это очень помогло мне придумать интерфейсы, которые «принадлежат» потребителю, а не поставщику . Поэтому подумайте: « Мне как потребителю нужно то, что делает то-то и то-то», а не как поставщику, я могу делать то-то и то-то ».
Корак

Ответы:

31

Нет абсолютно ничего плохого в передаче целого Userобъекта в качестве параметра. Фактически, это может помочь прояснить ваш код и сделать более понятным для программистов, что метод берет, если сигнатура метода требует a User.

Передавать простые типы данных приятно, пока они не означают что-то другое, чем они есть. Рассмотрим этот пример:

public class Foo
{
    public void Bar(int userId)
    {
        // ...
    }
}

И пример использования:

var user = blogPostRepository.Find(32);
var foo = new Foo();

foo.Bar(user.Id);

Вы можете определить дефект? Компилятор не может. Передаваемый «идентификатор пользователя» является целым числом. Мы называем переменную, userно инициализируем ее значение из blogPostRepositoryобъекта, который, по-видимому, возвращает BlogPostобъекты, а не Userобъекты - но код компилируется, и в результате вы получаете неожиданную ошибку времени выполнения.

Теперь рассмотрим этот измененный пример:

public class Foo
{
    public void Bar(User user)
    {
        // ...
    }
}

Возможно, Barметод использует только «идентификатор пользователя», но для подписи метода требуется Userобъект. Теперь давайте вернемся к тому же примеру использования, что и раньше, но изменим его, чтобы передать всего «пользователя»:

var user = blogPostRepository.Find(32);
var foo = new Foo();

foo.Bar(user);

Теперь у нас есть ошибка компилятора. blogPostRepository.FindМетод возвращает BlogPostобъект, который мы называем хитро «пользователь». Затем мы передаем этого «пользователя» в Barметод и быстро получаем ошибку компилятора, потому что мы не можем передать a BlogPostв метод, который принимает a User.

Система типов языка используется для быстрого написания правильного кода и выявления дефектов во время компиляции, а не во время выполнения.

Действительно, необходимость рефакторинга большого количества кода, потому что пользовательская информация изменяется, является просто симптомом других проблем. Передавая весь Userобъект, вы получаете преимущества, описанные выше, в дополнение к преимуществам отсутствия необходимости рефакторинга всех сигнатур методов, которые принимают пользовательскую информацию, когда что-то в Userклассе изменяется.

Грег Бургхардт
источник
6
Я бы сказал, что ваши рассуждения на самом деле указывают на прохождение полей, но наличие полей является тривиальной оберткой вокруг реального значения. В этом примере Пользователь имеет поле типа UserID, а UserID имеет одно целочисленное поле. Теперь объявление Bar немедленно сообщает вам, что Bar не использует всю информацию о Пользователе, только его ID, но вы все равно не можете делать глупых ошибок, таких как передача целого числа, которое не пришло из UserID, в Bar.
Ян
(Продолжение.) Конечно, этот стиль программирования довольно утомителен, особенно в языке, который не имеет хорошей синтаксической поддержки для него (например, Haskell хорош для этого стиля, так как вы можете просто сопоставить его по «идентификатору пользователя») ,
Ян
5
@Ian: Я думаю, что обертывание Id в своем собственном типе скатится вокруг исходной проблемы, поднятой OP, которая заключается в структурных изменениях в классе User, которые требуют рефакторинга многих сигнатур методов. Передача всего объекта User решает эту проблему.
Грег Бургхардт
@Ian: Хотя, честно говоря, даже работая в C #, я очень соблазнялся обернуть идентификаторы и тому подобное в Struct, просто чтобы дать немного больше ясности.
Грег Бургхардт
1
«нет ничего плохого в том, чтобы навести указатель на свое место». Или ссылка, чтобы избежать всех проблем с указателями, с которыми вы можете столкнуться.
Yay295
17

Правильно ли я считаю, что это нарушение принципа Open / Closed?

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

Какие-либо другие принципы нарушаются этой практикой? Зависимость инверсии perahaps?

Нет. То, что вы описываете - вводите только необходимые части пользовательского объекта в каждый метод - наоборот: это чистая инверсия зависимостей.

Существуют ли другие, не твердые принципы, которые нарушаются, такие как базовые принципы защитного программирования?

Нет. Этот подход является вполне допустимым способом кодирования. Это не нарушает такие принципы.

Но инверсия зависимости - это только принцип; это не нерушимый закон. И чистый DI может добавить сложности в систему. Если вы обнаружите, что только введение нужных пользовательских значений в методы, а не передача всего пользовательского объекта в метод или конструктор, создает проблемы, то не делайте так. Это все о достижении баланса между принципами и прагматизмом.

Чтобы ответить на ваш комментарий:

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

Отчасти проблема заключается в том, что вам явно не нравится этот подход, согласно комментарию «излишне [пройти] ...». И это достаточно справедливо; здесь нет правильного ответа. Если вы находите это обременительным, не делайте так.

Однако, что касается принципа открытого / закрытого, если вы строго следуете этому, то «... изменить все ссылки на все пять из этих существующих методов ...» означает, что эти методы были изменены, когда они должны быть закрыто для модификации. В действительности, хотя принцип open / closed имеет смысл для общедоступных API, но не имеет особого смысла для внутренних компонентов приложения.

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

Но тогда вы забредаете на территорию ЯГНИ, и это все равно будет ортогонально принципу. Если у вас есть метод, Fooкоторый принимает имя пользователя, а затем вы также хотите Fooуказать дату рождения, следуя этому принципу, вы добавляете новый метод; Fooостается неизменной. Опять же, это хорошая практика для публичных API, но это нонсенс для внутреннего кода.

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

Дэвид Арно
источник
Существуют проблемы, связанные с необязательным анализом нового значения по пяти уровням цепочки, а затем изменением всех ссылок на все пять из этих существующих методов. Почему принцип Open / Closed применим только к классу User, а не к классу, который я сейчас редактирую, который также используется другими классами? Я знаю, что этот принцип конкретно касается избежания изменений, но, безусловно, план соблюдения этого принципа, насколько это практически возможно, будет включать стратегии, направленные на уменьшение необходимости будущих изменений?
Джимбо
@ Джимбо, я обновил свой ответ, чтобы попытаться ответить на ваш комментарий.
Дэвид Арно,
Я ценю ваш вклад. КСТАТИ. Даже Роберт С. Мартин не признает, что принцип Open / Closed имеет жесткое правило. Это правило, которое неизбежно будет нарушено. Применение принципа - это упражнение в попытках соблюдать его настолько, насколько это практически возможно. Вот почему я использовал слово «практически осуществимо» ранее.
Джимбо
Это не инверсия зависимостей для передачи параметров пользователя, а не самого пользователя.
Джеймс Эллис-Джонс
@ JamesEllis-Jones, инверсия зависимостей переворачивает зависимости от «спросить» до «сказать». Если вы передаете Userэкземпляр и затем запрашиваете этот объект, чтобы получить параметр, то вы только частично инвертируете зависимости; все еще есть некоторые вопросы. Истинная инверсия зависимостей на 100% «говори, не спрашивай». Но это приходит по цене сложности.
Дэвид Арно
10

Да, изменение существующей функции является нарушением принципа Open / Closed. Вы модифицируете что-то, что должно быть закрыто для модификации из-за изменения требований. Лучшим дизайном (чтобы не измениться при изменении требований) было бы передать пользователю то, что должно работать на пользователей.

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

Так что, как и с большинством вещей - это зависит .

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

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

Telastyn
источник
+1, но я не уверен в вашей фразе «вы могли бы передавать больше информации». Когда вы передаете (User theuser) вы передаете минимум информации, ссылку на один объект. Правда, эту ссылку можно использовать для получения дополнительной информации, но это означает, что вызывающий код не должен ее получать. Когда вы передаете (GUID userid, string username) вызываемый метод всегда может вызвать User.find (userid), чтобы найти открытый интерфейс объекта, так что вы ничего не скрываете.
18:00
5
@dcorking, « Когда вы передаете (User theuser) вы передаете минимум информации, ссылку на один объект ». Вы передаете максимум информации, связанной с этим объектом: весь объект. msgstr " вызываемый метод всегда может вызвать User.find (userid) ...". В хорошо спроектированной системе это было бы невозможно, поскольку у рассматриваемого метода не было бы доступа User.find(). На самом деле даже не должно бытьUser.find . Поиск пользователя никогда не должен быть обязанностью User.
Давид Арно,
2
@ Dcorking - Enh. То, что вы передаете ссылку, которая оказывается маленькой, является техническим совпадением. Вы связываете все Userс функцией. Может быть, это имеет смысл. Но, возможно, функция должна заботиться только об имени пользователя - и передавать такие вещи, как дата присоединения пользователя или адрес, является неправильным.
Теластин
@ ДавидАрно, возможно, это ключ к четкому ответу на ОП. Чьей ответственностью должен быть поиск Пользователя? Существует ли название для принципа разделения отделителя / фабрики от класса?
18:00
1
@dcorking Я бы сказал, что это одно из следствий принципа единой ответственности. Знание, где хранятся пользователи и как получить их по идентификатору, - это отдельные обязанности, которые Userне должны иметь классы. Там может быть UserRepositoryили похожий, который имеет дело с такими вещами.
Халк
3

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

Правильно ли я считаю, что это нарушение принципа Open / Closed?

Нет. Применение этого шаблона активирует принцип открытия / закрытия (OCP). Например, производные классы Userмогут быть предоставлены в качестве параметра, который вызывает другое поведение в потребляющем классе.

Какие-либо другие принципы нарушаются этой практикой?

Это может случиться Позвольте мне объяснить на основе твердых принципов.

Принцип единой ответственности (SRP) может быть нарушен, если он имеет дизайн, как вы объяснили:

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

Проблема со всей информацией . Если у Userкласса много свойств, он становится огромным объектом передачи данных, который транспортирует несвязанную информацию с точки зрения потребляющих классов. Пример: с точки зрения класса-потребителя UserAuthenticationэто свойство User.Idи User.Nameуместно, но нет User.Timezone.

Принцип интерфейса сегрегации (ISP) также нарушается с рассуждениями , но похожими добавляет другую перспективу. Пример: предположим, что потребляющий класс UserManagementтребует, чтобы свойство User.Nameбыло разделено до, User.LastNameи User.FirstNameкласс также UserAuthenticationдолжен быть изменен для этого.

К счастью, интернет-провайдер также дает вам выход из проблемы: обычно такие объекты параметров или объекты транспорта данных начинаются с малого и растут со временем. Если это становится громоздким, рассмотрите следующий подход: представьте интерфейсы, адаптированные к потребностям потребляющих классов. Пример: представьте интерфейсы и позвольте Userклассу наследовать от него:

class User : IUserAuthenticationInfo, IUserLocationInfo { ... }

Каждый интерфейс должен предоставлять подмножество связанных свойств Userкласса, необходимых для потребляющего класса, чтобы он полностью выполнял свою работу. Ищите кластеры свойств. Попробуйте повторно использовать интерфейсы. В случае использования класса UserAuthenticationпотребления IUserAuthenticationInfoвместо User. Затем, если возможно, разбейте Userкласс на несколько конкретных классов, используя интерфейсы как «трафарет».

Тео Ленндорф
источник
1
Когда пользователь усложняется, возникает комбинаторный взрыв возможных подынтерфейсов, например, если у пользователя всего 3 свойства, есть 7 возможных комбинаций. Ваше предложение звучит хорошо, но не работает.
user949300
1. Аналитически вы правы. Однако в зависимости от того, как моделируется домен, биты связанной информации имеют тенденцию к кластеризации. Так что практически не нужно иметь дело со всеми возможными комбинациями интерфейсов и свойств. 2. Изложенный подход не был предназначен для универсального решения, но, возможно, я должен добавить еще несколько «возможно» и «можно» в ответ.
Тео Ленндорф
2

Столкнувшись с этой проблемой в моем собственном коде, я пришел к выводу, что базовые классовые модели / объекты являются ответом.

Типичным примером может служить шаблон хранилища. Часто при запросах к базе данных через репозитории многие методы в репозитории принимают множество одинаковых параметров.

Мои эмпирические правила для репозиториев:

  • Если более чем один метод принимает одинаковые 2 или более параметров, параметры должны быть сгруппированы как объект модели.

  • Если метод принимает более 2 параметров, параметры должны быть сгруппированы как объект модели.

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


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

И да, совершенно нормально иметь 2 разные модели с одинаковыми свойствами, которые служат разным слоям / целям (т. Е. ViewModels против POCO).

Дом
источник
2

Давайте просто проверим отдельные аспекты SOLID:

  • Одиночная ответственность: возможно, это будет нарушено, если люди будут проходить мимо только разделов класса.
  • Открыто / закрыто: не имеет значения, где разделы класса передаются, только когда полный объект передается. (Я думаю, что тут начинается когнитивный диссонанс: вам нужно изменить далекий код, но сам класс кажется нормальным.)
  • Замена Лискова: не проблема, мы не делаем подклассы.
  • Инверсия зависимостей (зависит от абстракций, а не от конкретных данных). Да, это нарушено: у людей нет абстракций, они вынимают конкретные элементы класса и раздают это. Я думаю, что это главная проблема здесь.

Одна вещь, которая имеет тенденцию сбивать с толку инстинкты проектирования, заключается в том, что этот класс предназначен главным образом для глобальных объектов и, по существу, только для чтения. В такой ситуации нарушение абстракций не причиняет большого вреда: простое чтение данных, которые не были изменены, создает довольно слабую связь; только когда она становится огромной кучей, боль становится заметной.
Чтобы восстановить проектные инстинкты, просто предположите, что объект не очень глобален. Какой контекст понадобится функции, если Userобъект может быть изменен в любое время? Какие компоненты объекта могут быть видоизменены вместе? Они могут быть выделены Userкак в виде подобъекта, на который ссылаются, или в качестве интерфейса, который предоставляет только «часть» связанных полей, не так уж важно.

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

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

Разделение Userна самом деле станет уродливым, если субобъекты перекрываются, тогда люди будут не уверены, какой из них выбрать, если все обязательные поля находятся в перекрытии. Если вы разделитесь иерархически (например, у вас есть то, UserMarketSegmentчто, помимо прочего UserLocation), люди не будут уверены, на каком уровне находится функция, которую они пишут: имеет ли дело пользовательские данные на Location уровне или на MarketSegmentуровне? Точно не помогает, что это может измениться со временем, то есть вы вернулись к изменению сигнатур функций, иногда по всей цепочке вызовов.

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

Toolforger
источник
1

Это действительно интересный вопрос. Это зависит.

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

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

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

Джеймс Эллис-Джонс
источник
1

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

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

interface IIdentifieable
{
    Guid ID { get; }
}

или

interface INameable
{
    string Name { get; }
}

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

t3chb0t
источник
1

Вот что я встречал время от времени:

  • Метод принимает аргумент типа User(или Productлюбого другого), который имеет множество свойств, хотя метод использует только несколько из них.
  • По какой-то причине некоторая часть кода должна вызывать этот метод, даже если он не имеет полностью заполненного Userобъекта. Он создает экземпляр и инициализирует только те свойства, которые действительно нужны методу.
  • Это случается много раз.
  • Через некоторое время, когда вы сталкиваетесь с методом, у которого есть Userаргумент, вы обнаруживаете, что вынуждены находить вызовы этого метода, чтобы найти источник, Userчтобы вы знали, какие свойства заполнены. Это «реальный» пользователь с адресом электронной почты, или он просто создан для передачи идентификатора пользователя и некоторых разрешений?

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

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

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

Такое повреждение не является неизбежным, но оно происходит снова и снова, когда мы передаем объект только потому, что нам нужен доступ к нескольким его свойствам. Опасная зона - это первый случай, когда вы видите, как кто-то создает экземпляр Userи просто заполняет несколько свойств, чтобы они могли передать его методу. Положи свою ногу на нее, потому что это темный путь.

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

Скотт Ханнен
источник