Использует "плохую" практику

Question

Я только что добавил out bool параметр в метод, который я написал, чтобы вывести предупреждение в мой пользовательский интерфейс. Я использовал метод out вместо получения самого метода для возврата false / true, поскольку это означало бы, что DoSomething не удалось / удалось. Я думал, что warnUser будет указывать, что на самом деле было предупреждение, без необходимости смотреть на реализацию метода.

Оригинальный код

public void DoSomething(int id, string input);

Новый код

public void DoSomething(int id, string input, out bool warnUser);

Я использую Moq для тестирования этого кода, но он не поддерживает параметры out / ref, потому что они не поддерживаются лямбда-выражениями

Тестовый код

mockService.Verify(It.IsAny<int>(), It.IsAny<string>(), It.IsAny<bool>());

Итак, использование out параметров плохая практика и если да, то что мне делать вместо этого?

Answer 1

Использование параметра out в методе void обычно является плохой идеей.Вы говорите, что использовали его «вместо того, чтобы заставить сам метод возвращать false / true, поскольку это означало бы, что DoSomething потерпел неудачу / преуспел» - я не верю, что это имеет место.Обычно в .NET сбой указывается с помощью исключения, а не истины / ложи.

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

if (DoSomething(...))
{
   // Warn user here
}

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

public enum WarningLevel
{
    NoWarningRequired,
    WarnUser
}

Тогда метод может вернуть WarningLevel вместо bool.Это бы прояснило, что вы имели в виду - хотя вы можете слегка переименовать вещи.(Трудно дать совет с метасинтаксическими именами, такими как «DoSomething», хотя я полностью понимаю, почему вы использовали это здесь.)

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

Answer 2

out является очень полезной конструкцией, в частности в таких шаблонах, как bool TryGetFoo(..., out value), где вы хотите знать «если» и «что» отдельно (и nullable не обязательно является опцией).

Однако - в этом случае, почему бы просто не сделать это:

public bool DoSomething(int id, string input);

и использовать возвращаемое значение, чтобы сигнализировать это?

Answer 3

Несколько дополнительных мыслей:

• Что говорит FxCop: CA1021: избегать параметров

• Для частных / внутренних методов (подробности реализации) out / ref параметры менее важны

• C # 7 Кортежи часто являются лучшей альтернативой out параметрам

• Кроме того, C # 7 улучшает обработку параметра out на сайте вызова, вводя "переменные out" и позволяя "отбрасывать" out параметры

Answer 4

Учитывая большой устаревший метод кода (скажем, 15+ строк, устаревшие методы кода с 200+ строками довольно распространены), можно использовать рефакторинг «Extract Method», чтобы очистить и сделать код менее хрупким и более обслуживаемым.

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

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

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

Таким образом, в таком сценарии, выходной параметрRS, на мой взгляд, код, нарушающий SRP.

Answer 5

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

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

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

Answer 6

Я думаю, что лучшим решением для вашего случая является использование делегата вместо bool. Используйте что-то вроде этого:

public void DoSomething(int id, string input, Action warnUser);

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

Answer 7

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

public class MyWarningException : Exception() {}

...

try
{ 
     obj.DoSomething(id,input);
}
catch (MyWarningException ex)
{
     // do stuff
}

Идея состоит в том, что исключение возникает в конце реализации DoSomething, чтобы поток не останавливался в середине