Validación de parámetros de constructor en C # - Mejores prácticas

34

¿Cuál es la mejor práctica para la validación de parámetros de constructor?

Supongamos un simple bit de C #:

public class MyClass
{
    public MyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
            throw new ArgumentException("Text cannot be empty");

        // continue with normal construction
    }
}

¿Sería aceptable lanzar una excepción?

La alternativa que encontré fue la validación previa, antes de instanciar:

public class CallingClass
{
    public MyClass MakeMyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
        {
            MessageBox.Show("Text cannot be empty");
            return null;
        }
        else
        {
            return new MyClass(text);
        }
    }
}
MPelletier
fuente
10
"aceptable para lanzar una excepción?" Cual es la alternativa?
S.Lott
10
@ S.Lott: No hagas nada. Establecer un valor predeterminado. Imprima una mesage en la consola / archivo de registro. Sonar una campana. Parpadea una luz.
FrustratedWithFormsDesigner
3
@FrustratedWithFormsDesigner: "¿No hacer nada?" ¿Cómo se satisfará la restricción "texto no vacío"? "Establecer un valor predeterminado"? ¿Cómo se usará el valor del argumento de texto? Las otras ideas están bien, pero esas dos conducirían a un objeto en un estado que viola las restricciones de esta clase.
S.Lott
1
@ S.Lott Por miedo a repetirme, estaba abierto a la posibilidad de que hubiera otro enfoque, que no conocía. Creo que no todo lo sé, eso lo sé con certeza.
MPelletier
3
@MPelletier, validar los parámetros con anticipación terminará violando DRY. (No se repita) Como deberá copiar esa validación previa a cualquier código que intente crear una instancia de esta clase.
CaffGeek

Respuestas:

26

Tiendo a realizar toda mi validación en el constructor. Esto es imprescindible porque casi siempre creo objetos inmutables. Para su caso específico, creo que esto es aceptable.

if (string.IsNullOrEmpty(text))
    throw new ArgumentException("message", "text");

Si está utilizando .NET 4, puede hacerlo. Por supuesto, esto depende de si considera que una cadena que contiene solo espacios en blanco no es válida.

if (string.IsNullOrWhiteSpace(text))
    throw new ArgumentException("message", "text");
ChaosPandion
fuente
No estoy seguro de que su "caso específico" sea lo suficientemente específico como para dar un consejo tan específico. No tenemos idea de si tiene sentido en este contexto lanzar una excepción o simplemente permitir que el objeto exista de todos modos.
FrustratedWithFormsDesigner
@FrustratedWithFormsDesigner - ¿Supongo que me votaste en contra por esto? Por supuesto, tiene razón en que estoy extrapolando, pero claramente este objeto teórico debe ser inválido si el texto de entrada está vacío. ¿Realmente querría tener que llamar Initpara recibir algún tipo de código de error cuando una excepción funcione igual de bien y obligue a su objeto a mantener siempre un estado válido?
ChaosPandion
2
Tiendo a dividir ese primer caso en dos excepciones separadas: una que prueba la nulidad y arroja una ArgumentNullExceptiony otra que prueba el vacío y arroja una ArgumentException. Permite que la persona que llama sea más exigente si lo desea en su manejo de excepciones.
Jesse C. Slicer
1
@Jesse: he usado esa técnica, pero todavía no estoy seguro de su utilidad general.
ChaosPandion
3
¡+1 para objetos inmutables! También los uso siempre que sea posible (personalmente lo encuentro casi siempre).
22

Mucha gente dice que los constructores no deberían lanzar excepciones. KyleG en esta página , por ejemplo, hace exactamente eso. Honestamente, no puedo pensar en una razón por la que no.

En C ++, lanzar una excepción de un constructor es una mala idea, porque te deja con memoria asignada que contiene un objeto no inicializado al que no tienes referencia (es decir, es una pérdida de memoria clásica). Probablemente de ahí proviene el estigma: un grupo de desarrolladores de C ++ de la vieja escuela analizó a medias su aprendizaje de C # y simplemente aplicó lo que sabían de C ++. Por el contrario, en Objective-C Apple separó el paso de asignación del paso de inicialización, por lo que los constructores en ese lenguaje pueden lanzar excepciones.

C # no puede perder memoria de una llamada fallida a un constructor. Incluso algunas clases en .NET Framework arrojarán excepciones en sus constructores.

Hormiga
fuente
Vas a revolver algunas plumas con tu comentario de C ++. :)
ChaosPandion
55
Ehh, C ++ es un gran lenguaje, pero una de mis manías son las personas que aprenden bien un idioma y luego escriben así todo el tiempo . C # no es C ++.
Ant
10
Todavía puede ser peligroso lanzar excepciones de un ctor en C # porque el ctor podría haber asignado recursos no administrados que nunca se eliminarán. Y el finalizador debe escribirse para estar a la defensiva contra el escenario en el que se finaliza un objeto construido de forma incompleta. Pero estos escenarios son relativamente raros. Un próximo artículo sobre el Centro de desarrollo de C # cubrirá estos escenarios y cómo codificar defensivamente a su alrededor, así que mire ese espacio para obtener más detalles.
Eric Lippert
66
eh? lanzar una excepción de la ctor es el único que se puede hacer en C ++ si no se puede construir el objeto, y no hay ninguna razón de que esto tiene para causar una pérdida de memoria (aunque, obviamente, su hasta).
jk.
@jk: Hmm, tienes razón! Aunque parece que una alternativa es marcar los objetos malos como "zombies", lo que aparentemente hace el STL en lugar de lanzar excepciones: yosefk.com/c++fqa/exceptions.html#fqa-17.2 La solución de Objective-C es mucho más ordenada.
Ant
12

Lanzar una excepción IFF la clase no puede ponerse en un estado consistente con respecto a su uso semántico. De lo contrario no lo hagas. NUNCA permita que exista un objeto en un estado inconsistente. Esto incluye no proporcionar constructores completos (como tener un constructor vacío + initialize () antes de que su objeto esté realmente completamente construido) ... ¡SOLO DIGA NO!

En caso de apuro, todos lo hacen. Lo hice el otro día para un objeto de uso muy limitado dentro de un alcance estrecho. Algún día en el futuro, yo u otra persona probablemente pagaremos el precio de ese desliz en la práctica.

Debo señalar que por "constructor" me refiero a lo que el cliente llama para construir el objeto. Eso podría ser fácilmente algo más que una construcción real que se conoce con el nombre de "Constructor". Por ejemplo, algo así en C ++ no violaría el principio IMNSHO:

struct funky_object
{
  ...
private:
  funky_object();
  bool initialize(std::string);

  friend boost::optional<funky_object> build_funky(std::string);
};
boost::optional<funky_object> build_funky(std::string str)
{
  funky_object fo;
  if (fo.initialize(str)) return fo;
  return boost::optional<funky_object>();
}

Dado que la única forma de crear un funky_objectes llamando build_funkyal principio de nunca permitir que exista un objeto no válido, permanece intacto aunque el "Constructor" real no termine el trabajo.

Sin embargo, eso es mucho trabajo adicional para obtener ganancias cuestionables (tal vez incluso pérdidas). Todavía prefiero la ruta de excepción.

Edward extraño
fuente
9

En este caso, usaría el método de fábrica. Básicamente, establezca que su clase tenga solo constructores privados y tenga un método de fábrica que devuelva una instancia de su objeto. Si los parámetros iniciales no son válidos, simplemente devuelva nulo y haga que el código de llamada decida qué hacer.

public class MyClass
{
    private MyClass(string text)
    {
        //normal construction
    }

    public static MyClass MakeMyClass(string text)
    {
        if (String.IsNullOrEmpty(text))
            return null;
        else
            return new MyClass(text);
    }
}
public class CallingClass
{
    public MyClass MakeMyClass(string text)
    {
        var cls = MyClass.MakeMyClass(text);
        if(cls == null)
             //show messagebox or throw exception
        return cls;
    }
}

Nunca arroje excepciones a menos que las condiciones sean excepcionales. Estoy pensando que en este caso, un valor vacío se puede pasar fácilmente. Si ese es el caso, el uso de este patrón evitaría excepciones y las penalizaciones de rendimiento en las que incurren mientras mantiene válido el estado MyClass.

Donn Relacion
fuente
1
No veo la ventaja. ¿Por qué es preferible probar el resultado de una fábrica como nulo a tener un try-catch en algún lugar que pueda detectar la excepción del constructor? Su enfoque parece ser "hacer caso omiso de las excepciones, volver a verificar explícitamente el valor de retorno de los métodos para detectar errores". Hace mucho tiempo aceptamos que las excepciones son generalmente superiores a tener que probar explícitamente cada valor de retorno de método para detectar errores, por lo que su consejo parece sospechoso. Me gustaría ver alguna justificación de por qué crees que la regla general no se aplica aquí.
DW
nunca aceptamos que las excepciones fueran superiores a los códigos de retorno, pueden ser un éxito enorme si se lanzan con demasiada frecuencia. Sin embargo, lanzar una excepción desde un constructor perderá memoria, incluso en .NET, si ha asignado algo que no está administrado. Tienes que hacer mucho trabajo en tu finalizador para compensar este caso. De todos modos, la fábrica es una buena idea aquí, y TBH debería arrojar una excepción en lugar de devolver un valor nulo. Suponiendo que cree solo unas pocas clases 'malas', es preferible hacer el lanzamiento de fábrica.
gbjbaanb
2
  • Un constructor no debería tener ningún efecto secundario.
    • Cualquier cosa más que la inicialización de campo privado debe verse como un efecto secundario.
    • Un constructor con efectos secundarios rompe el Principio de Responsabilidad Única (SRP) y es contrario al espíritu de la programación orientada a objetos (OOP).
  • Un constructor debe ser ligero y nunca debe fallar.
    • Por ejemplo, siempre me estremezco cuando veo un bloque try-catch dentro de un constructor. Un constructor no debe lanzar excepciones o errores de registro.

Uno podría cuestionar razonablemente estas pautas y decir: "¡Pero no sigo estas reglas y mi código funciona bien!" A eso, respondería: "Bueno, eso puede ser cierto, hasta que no lo sea".

  • Excepciones y errores dentro de un constructor son muy inesperados. A menos que se les indique que lo hagan, los futuros programadores no estarán inclinados a rodear estas llamadas de constructor con código defensivo.
  • Si algo falla en la producción, el seguimiento de la pila generada puede ser difícil de analizar. La parte superior de la traza de la pila puede apuntar a la llamada del constructor, pero suceden muchas cosas en el constructor y puede que no apunte al LOC real que falló.
    • He analizado muchos rastros de pila .NET donde este fue el caso.
Jim G.
fuente
0

Depende de lo que esté haciendo MyClass. Si MyClass fuera en realidad una clase de repositorio de datos y el texto del parámetro fuera una cadena de conexión, la mejor práctica sería lanzar una excepción ArgumentException. Sin embargo, si MyClass es la clase StringBuilder (por ejemplo), entonces podría dejarlo en blanco.

Entonces, depende de cuán esencial sea el texto del parámetro para el método: ¿tiene sentido el objeto con un valor nulo o en blanco?

Steven Striga
fuente
2
Creo que la pregunta implica que la clase realmente requiere que la cadena no esté vacía. Ese no es el caso en su ejemplo StringBuilder.
Sergio Acosta
Entonces la respuesta debe ser sí: es aceptable lanzar una excepción. Para evitar tener que intentar / detectar este error, puede usar un patrón de fábrica para manejar la creación del objeto por usted, que incluiría un caso de cadena vacía.
Steven Striga
0

Prefiero establecer un valor predeterminado, pero sé que Java tiene la biblioteca "Apache Commons" que hace algo como esto, y creo que también es una buena idea. No veo un problema al lanzar una excepción si el valor no válido pondría el objeto en un estado inutilizable; una cadena no es un buen ejemplo, pero ¿y si fuera por la DI del pobre? No podría operar si nullse pasara un valor de en lugar de, digamos, una ICustomerRepositoryinterfaz. En una situación como esta, diría que lanzar una excepción es la forma correcta de manejar las cosas.

Wayne Molina
fuente
-1

Cuando solía trabajar en c ++, no solía lanzar una excepción en el constructor debido al problema de pérdida de memoria que puede provocar, lo había aprendido por las malas. Cualquiera que trabaje con c ++ sabe lo difícil y problemática que puede ser la pérdida de memoria.

Pero si está en c # / Java, entonces no tiene este problema, porque el recolector de basura recolectará la memoria. Si está utilizando C #, creo que es preferible utilizar la propiedad de una manera agradable y coherente para garantizar que las restricciones de datos estén garantizadas.

public class MyClass
{
    private string _text;
    public string Text 
    {
        get
        {
            return _text;
        } 
        set
        {
            if (string.IsNullOrWhiteSpace(value))
                throw new ArgumentException("Text cannot be empty");
            _text = value;
        } 
    }

    public MyClass(string text)
    {
        Text = text;
        // continue with normal construction
    }
}
ajitdh
fuente
-3

Lanzar una excepción será un verdadero dolor para los usuarios de tu clase. Si la validación es un problema, generalmente hago que la creación del objeto sea un proceso de 2 pasos.

1 - Crear la instancia

2 - Llame a un método Initialize con un resultado de retorno.

O

Llame a un método / función que cree la clase para usted que pueda hacer la validación.

O

Tiene una bandera isValid, que no me gusta especialmente, pero algunas personas sí.

Remojar
fuente
3
@Dunk, eso está mal.
CaffGeek
44
@Chad, esa es tu opinión, pero mi opinión no está mal. Es simplemente diferente al tuyo. Encuentro que el código try-catch es mucho más difícil de leer y he visto muchos más errores creados cuando las personas usan try-catch para el manejo de errores triviales que los métodos más tradicionales. Esa ha sido mi experiencia y no está mal. Es lo que es.
Dunk
55
El PUNTO es que después de llamar a un constructor, si no se crea porque una entrada no es válida, eso es una EXCEPCIÓN. Los datos de la aplicación ahora están en un estado inconsistente / inválido si simplemente creo el objeto y configuro un indicador que dice isValid = false. Tener que probar después de crear una clase si es válido es horrible, un diseño muy propenso a errores. Y, usted dijo, tenga un constructor, luego llame a initialize ... ¿Qué pasa si no llamo initialize? ¿Entonces que? ¿Y qué pasa si Initialize obtiene datos incorrectos, puedo lanzar una excepción ahora? Tu clase no debería ser difícil de usar.
CaffGeek
55
@Dunk, si encuentra excepciones difíciles de usar, las está usando mal. En pocas palabras, se proporcionó información incorrecta a un constructor. Eso es una excepción. La aplicación no debe continuar ejecutándose a menos que se solucione. Período. Si lo hace, terminará con un problema peor, datos incorrectos. Si no sabe cómo manejar la excepción, no lo sabe, deje que suba la pila de llamadas hasta que pueda manejarse, incluso si eso significa simplemente iniciar sesión y detener la ejecución. En pocas palabras, no necesita manejar excepciones o probarlas. Ellos simplemente trabajan.
CaffGeek
3
Lo sentimos, esto es demasiado antipatrón para considerar.
MPelletier