Мы проводим много модульного тестирования и рефакторинга наших бизнес-объектов, и у меня, похоже, совсем другие мнения о дизайне классов, чем у других коллег.
Пример класса, который я не фанат:
public class Foo
{
private string field1;
private string field2;
private string field3;
private string field4;
private string field5;
public Foo() { }
public Foo(string in1, string in2)
{
field1 = in1;
field2 = in2;
}
public Foo(string in1, string in2, string in3, string in4)
{
field1 = in1;
field2 = in2;
field3 = in3;
}
public Prop1
{ get { return field1; } }
{ set { field1 = value; } }
public Prop2
{ get { return field2; } }
{ set { field2 = value; } }
public Prop3
{ get { return field3; } }
{ set { field3 = value; } }
public Prop4
{ get { return field4; } }
{ set { field4 = value; } }
public Prop5
{ get { return field5; } }
{ set { field5 = value; } }
}
В «реальном» классе они не все строковые, но в некоторых случаях у нас есть 30 вспомогательных полей для полностью открытых свойств.
Я ненавижу этот класс, и я не знаю, привередничаю ли я. Несколько примечаний:
- Частные поля поддержки без логики в свойствах, кажутся ненужными и раздувают класс
- Несколько конструкторов (несколько хорошо), но в сочетании с
- все свойства, имеющие открытый сеттер, я не фанат.
- Потенциально никаким свойствам не будет присвоено значение из-за пустого конструктора, если вызывающий объект не знает, вы можете получить некоторые очень нежелательные и сложные для проверки поведения.
- Это слишком много свойств! (в 30 случае)
Я нахожу гораздо сложнее действительно понять, в каком состоянии находится объект Foo
в любой момент времени, как реализатор. Был выдвинут аргумент: «У нас может не быть необходимой информации для установки Prop5
во время создания объекта. Хорошо, я думаю, я могу это понять, но если это так, сделайте Prop5
общедоступным только сеттер, а не всегда до 30 свойств в классе.
Я просто придирчив и / или сумасшедший из-за того, что мне нужен класс, который «прост в использовании», а не «легко писать (все публично)»? Классы, подобные приведенным выше, кричат мне, я не знаю, как это будет использоваться, поэтому я просто собираюсь обнародовать все на всякий случай .
Если я не очень требователен, каковы хорошие аргументы для борьбы с этим типом мышления? Я не очень хорош в формулировании аргументов, потому что я очень расстроен, пытаясь донести свою точку зрения (не намеренно, конечно).
get
илиset
:-)Ответы:
Полностью публичные классы имеют оправдание для определенных ситуаций, а также других крайних классов с одним открытым методом (и, вероятно, множеством частных методов). И классы с некоторыми публичными, а также с некоторыми приватными методами.
Все зависит от типа абстракции, которую вы собираетесь смоделировать с ними, какие уровни у вас есть в вашей системе, степень инкапсуляции, которая вам нужна на разных уровнях, и (конечно) какая школа мысли придет к автору класса от. Вы можете найти все эти типы в твердом коде.
Есть целые книги, написанные о том, когда отдавать предпочтение какому типу дизайна, поэтому я не собираюсь перечислять здесь какие-либо правила, места в этом разделе будет недостаточно. Однако, если у вас есть реальный пример абстракции, которую вы любите моделировать с помощью класса, я уверен, что сообщество с радостью поможет вам улучшить дизайн.
Для решения других ваших вопросов:
Так что вместо
записывать
или
«Несколько конструкторов»: это не проблема сама по себе. Это становится проблемой, когда возникает ненужное дублирование кода, как показано в вашем примере, или когда иерархия вызовов запутана. Это может быть легко решено путем рефакторинга общих частей в отдельную функцию и путем организации цепочки конструктора однонаправленным образом.
«Потенциально никаким свойствам не будет присвоено значение из-за пустого конструктора»: в C # каждый тип данных имеет четко определенное значение по умолчанию. Если свойства не инициализируются явно в конструкторе, им присваивается это значение по умолчанию. Если это используется намеренно , это совершенно нормально, поэтому пустой конструктор может быть в порядке, если автор знает, что он делает.
«Это слишком много свойств! (В случае с 30)»: да, если вы можете разработать такой класс с нуля, 30 - это слишком много, я согласен. Однако не у каждого из нас есть такая роскошь (разве вы не написали в комментарии ниже, что это устаревшая система?). Иногда вам необходимо отобразить записи из существующей базы данных, файла или данных стороннего API в вашу систему. Таким образом, для этих случаев 30 атрибутов могут быть чем-то, с чем нужно жить.
источник
null
это не так? так что если одно из свойств, которое фактически используется в a.ToUpper()
, но для него никогда не задано значение, это вызовет исключение времени выполнения. Это еще один хороший пример , почему необходимые данные для класса должны иметь быть установлены во время строительства объекта. Не просто оставить это на усмотрение пользователя. СпасибоГоворя, что «Частные вспомогательные поля без логики в свойствах кажутся ненужными и раздувают класс», у вас уже есть достойный аргумент.
Не похоже, что несколько конструкторов являются проблемой здесь.
Что касается того, чтобы все свойства были общедоступными ... Возможно, подумайте об этом так, если вы когда-нибудь захотите синхронизировать доступ ко всем этим свойствам между потоками, вам будет непросто, потому что вам, возможно, придется писать код синхронизации везде, где они используемый. Однако, если бы все свойства были заключены в getter / setters, вы могли бы легко встроить синхронизацию в класс.
Я думаю, что когда вы говорите «трудно проверить поведение», аргумент говорит сам за себя. Ваш тест, возможно, должен проверить, не является ли он нулевым или что-то в этом роде (каждое отдельное место, где используется свойство). Я действительно не знаю, потому что я не знаю, как выглядит ваш тест / приложение.
Вы правы слишком много свойств, не могли бы вы использовать наследование (если свойства достаточно похожи), а затем получить список / массив с использованием обобщений? Тогда вы могли бы написать методы получения / установки для доступа к информации и упростить взаимодействие со всеми свойствами.
источник
Как вы мне сказали,
Foo
это бизнес-объект. Он должен инкапсулировать некоторое поведение в методах в соответствии с вашим доменом, а его данные должны оставаться максимально инкапсулированными.В примере, который вы показываете, он
Foo
больше похож на DTO и идет вразрез со всеми принципами ООП (см. Модель анемичного домена )!В качестве улучшений я бы предложил:
Это может быть потенциальный рефакторинг класса с этими замечаниями:
источник
«Теперь мы можем выполнить модульное тестирование этих рефакторинговых методов. Так как по причинам x, y, z клиент не тестируется.
В той степени, в которой любой из приведенных выше аргументов может быть приведен в совокупности:
источник