Está usando una mala práctica "fuera de servicio"

Question

Acabo de agregar un parámetro bool a un método que he escrito para recibir una advertencia en mi interfaz de usuario. He utilizado una salida en lugar de obtener el método en sí para devolver falso/verdadero ya que implicaría que el DoSomething ha fallado/tenido éxito. Mi opinión era que el warnUser indicaría cuál era realmente la advertencia sin tener que observar la implementación del método.

Código Original

public void DoSomething(int id, string input); 

Nuevo Código

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

estoy usando Moq a probar este código, pero no soporta a cabo parámetros/ref porque son no compatible con las expresiones de Lambda

Código de prueba

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

Por lo tanto, está utilizando fuera parámetros mala práctica y si es así, ¿qué hago en su lugar?

Answer 1

El uso de un parámetro de salida en un método vacío es generalmente una mala idea. Dice que lo ha usado "en lugar de hacer que el método en sí sea falso/verdadero, ya que implicaría que DoSomething falló/tuvo éxito". No creo que la implicación esté ahí. Por lo general, en .NET falla se indica a través de una excepción en lugar de verdadero/falso.

out parámetros son generalmente más feo de usar que los valores de retorno - en particular, usted tiene que tener una variable del tipo adecuado para manejar, por lo que no sólo se puede escribir:

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

Una alternativa que podría quiero considerar es una enumeración que indica el nivel de advertencia requerido. Por ejemplo:

public enum WarningLevel 
{ 
    NoWarningRequired, 
    WarnUser 
} 

A continuación, el método podría devolver un WarningLevel en lugar de bool. Eso aclararía lo que querías decir, aunque es posible que quieras cambiar el nombre ligeramente. (Es difícil dar consejos con nombres metasintácticos como "DoSomething" aunque entiendo completamente por qué lo has usado aquí.)

Por supuesto, otra alternativa es que quieras que haya más información presente, como el razón para la advertencia. Eso podría hacerse con una enumeración, o quizás desee dar un resultado más completo por completo.

Answer 2

algunas reflexiones adicionales:

• Lo FxCop dice: CA1021: Avoid out parameters

• Para los métodos privados/interna (los detalles de implementación) out/ref parámetros son un problema menor

• C# 7 Tuples a menudo son una mejor alternativa a out parámetros

• Además, C# 7 mejora el manejo de out parámetro en el sitio de llamadas mediante la introducción de "variables out" y permitiendo a "descartar" out parámetros

Answer 3

Creo que la mejor manera de hacer esto es utilizar Excepciones esperadas. Puede crear un conjunto de excepciones personalizadas que definan sus advertencias y captarlas en el método de llamada.

public class MyWarningException : Exception() {} 

... 

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

La idea sería que la excepción se eleva al final de la ejecución HacerAlgo, por lo que el flujo no se detiene en el medio

Answer 4

out es mucho una construcción útil, en particular, en los patrones como bool TryGetFoo(..., out value), en la que desea conocer el "si" y el "qué" separado (y anulable no es necesariamente una opción).

Sin embargo - en este caso, ¿por qué no hacerlo:

public bool DoSomething(int id, string input); 

y utilizar el valor de retorno para indicar esto?

Answer 5

Esta es una pregunta realmente difícil de responder sin saber más sobre el contexto.

Si DoSomething es un método en la capa de la interfaz de usuario que hace algo relacionado con la interfaz de usuario, entonces quizás esté bien. Sin embargo, si DoSomething es un método de capa de negocios, probablemente este no sea un buen enfoque, ya que implica que la capa de negocios necesita comprender qué respuesta adecuada podría ser, e incluso podría ser consciente si hay problemas de localización.

En una nota puramente subjetiva, he tendido a mantenerme alejado de los parámetros. Creo que interrumpen el flujo del código y lo hacen un poco menos claro.

Answer 6

Creo que la mejor solución para su caso es utilizar un delegado en lugar de bool. Usar algo como esto:

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

se puede pasar un lambda vacío en el que no desea mostrar advertencias a los usuarios.

Answer 7

Dado un gran método de código heredado (digamos más de 15 líneas, métodos de código heredado con más de 200 líneas), se puede utilizar la refactorización "Extraer método" para limpiar y hacer que el código sea menos frágil y más fácil de mantener .

Tal refactorización es muy probable que produzca uno o más métodos nuevos sin parámetros para establecer valores de variables locales al método anterior.

Personalmente, utilizo esta circunstancia como un fuerte indicador de que ha habido una o más clases descrete escondidas en el método anterior, de modo que puedo usar "Extraer clase", donde en la nueva clase esos parámetros tienen propiedades visibles al método de llamada (anterior).

Considero que es una mala práctica dejar los métodos extraídos sin parámetros en el método anterior, omitiendo el siguiente paso coherente de "Extraer clase", porque si lo omite, esas variables locales permanecen en su función anterior, pero lo hacen semánticamente ya no le pertenecen.

Entonces, en tal escenario, los parámetros en mi opinión son un código que rompe SRP.