2010-11-23 3 views
7

Я только что добавил 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, чтобы проверить этот код, но он не поддерживает из/исх параметры, потому что они не поддерживается лямбда-выражениями

Тестовый код

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

Таким образом, использует Out параметры плохой практики и если да, что делать вместо этого?

+1

Как «bool» указывает, что на самом деле было предупреждением? » – 2010-11-23 10:49:16

+0

@Cody - потому что это значимое имя в моем «реальном» коде :) – 2010-11-23 10:58:08

ответ

9

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

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

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

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

public enum WarningLevel 
{ 
    NoWarningRequired, 
    WarnUser 
} 

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

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

3

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

  • Что FxCop говорит: CA1021: Avoid out parameters

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

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

  • Далее C# 7 улучшает обработку out параметра на месте вызова путем введения «из переменных» и позволяя «сбросить» out параметры

0

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

public class MyWarningException : Exception() {} 

... 

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

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

+5

Исключение должно быть выбрано, если метод не может сделать то, что он предназначен, IMO. Если это сработало, но по какой-то причине пользователю должно быть предоставлено предупреждение, это не похоже на исключительную ситуацию для меня. – 2010-11-23 10:50:45

+0

Исключение системы полезно, но очень медленно ... – ykatchou 2010-11-23 10:50:53

7

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

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

public bool DoSomething(int id, string input); 

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

0

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

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

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

0

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

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

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

1

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

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

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

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

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

Смежные вопросы