¿Hay una manera más fácil de probar la validación de argumentos y la inicialización de campo en un objeto inmutable?

20

Mi dominio consta de muchas clases simples inmutables como esta:

public class Person
{
    public string FullName { get; }
    public string NameAtBirth { get; }
    public string TaxId { get; }
    public PhoneNumber PhoneNumber { get; }
    public Address Address { get; }

    public Person(
        string fullName,
        string nameAtBirth,
        string taxId,
        PhoneNumber phoneNumber,
        Address address)
    {
        if (fullName == null)
            throw new ArgumentNullException(nameof(fullName));
        if (nameAtBirth == null)
            throw new ArgumentNullException(nameof(nameAtBirth));
        if (taxId == null)
            throw new ArgumentNullException(nameof(taxId));
        if (phoneNumber == null)
            throw new ArgumentNullException(nameof(phoneNumber));
        if (address == null)
            throw new ArgumentNullException(nameof(address));

        FullName = fullName;
        NameAtBirth = nameAtBirth;
        TaxId = taxId;
        PhoneNumber = phoneNumber;
        Address = address;
    }
}

Escribir las verificaciones nulas y la inicialización de propiedades ya se está volviendo muy tedioso, pero actualmente escribo pruebas unitarias para cada una de estas clases para verificar que la validación de argumentos funciona correctamente y que todas las propiedades se inicializan. Esto se siente como un trabajo extremadamente aburrido con beneficios inconmensurables.

La solución real sería que C # admitiera inmutabilidad y tipos de referencia no anulables de forma nativa. Pero, ¿qué puedo hacer para mejorar la situación mientras tanto? ¿Vale la pena escribir todas estas pruebas? ¿Sería una buena idea escribir un generador de código para tales clases para evitar escribir pruebas para cada una de ellas?


Esto es lo que tengo ahora basado en las respuestas.

Podría simplificar las verificaciones nulas y la inicialización de propiedades para que se vea así:

FullName = fullName.ThrowIfNull(nameof(fullName));
NameAtBirth = nameAtBirth.ThrowIfNull(nameof(nameAtBirth));
TaxId = taxId.ThrowIfNull(nameof(taxId));
PhoneNumber = phoneNumber.ThrowIfNull(nameof(phoneNumber));
Address = address.ThrowIfNull(nameof(address));

Usando la siguiente implementación de Robert Harvey :

public static class ArgumentValidationExtensions
{
    public static T ThrowIfNull<T>(this T o, string paramName) where T : class
    {
        if (o == null)
            throw new ArgumentNullException(paramName);

        return o;
    }
}

Probar las comprobaciones nulas es fácil usando GuardClauseAssertionfrom AutoFixture.Idioms(gracias por la sugerencia, Esben Skov Pedersen ):

var fixture = new Fixture().Customize(new AutoMoqCustomization());
var assertion = new GuardClauseAssertion(fixture);
assertion.Verify(typeof(Address).GetConstructors());

Esto podría comprimirse aún más:

typeof(Address).ShouldNotAcceptNullConstructorArguments();

Usando este método de extensión:

public static void ShouldNotAcceptNullConstructorArguments(this Type type)
{
    var fixture = new Fixture().Customize(new AutoMoqCustomization());
    var assertion = new GuardClauseAssertion(fixture);

    assertion.Verify(type.GetConstructors());
}
Botá Balázs
fuente
3
El título / etiqueta se refiere a la prueba (unidad) de los valores de campo que implica la prueba de la unidad posterior a la construcción del objeto, pero el fragmento de código muestra el argumento de entrada / validación de parámetros; Estos no son los mismos conceptos.
Erik Eidt
2
Para su información, podría escribir una plantilla T4 que facilitaría este tipo de repeticiones. (También puede considerar string.IsEmpty () beyond == null.)
Erik Eidt
1
¿Has echado un vistazo a los modismos de autofixture? nuget.org/packages/AutoFixture.Idioms
Esben Skov Pedersen
1
Relacionado: stackoverflow.com/questions/291340/…
Doc Brown
1
También podría usar Fody / NullGuard , aunque parece que no tiene un modo de permiso predeterminado.
Bob

Respuestas:

5

Creé una plantilla t4 exactamente para este tipo de casos. Para evitar escribir mucho repetitivo para clases inmutables.

https://github.com/xaviergonz/T4Immutable T4Immutable es una plantilla T4 para aplicaciones C # .NET que genera código para clases inmutables.

Hablando específicamente de pruebas no nulas, si usa esto:

[PreNotNullCheck, PostNotNullCheck]
public string FirstName { get; }

El constructor será este:

public Person(string firstName) {
  // pre not null check
  if (firstName == null) throw new ArgumentNullException(nameof(firstName));

  // assignations + PostConstructor() if needed

  // post not null check
  if (this.FirstName == null) throw new NullReferenceException(nameof(this.FirstName));
}

Dicho esto, si usa las anotaciones de JetBrains para la comprobación nula, también puede hacer esto:

[JetBrains.Annotations.NotNull, ConstructorParamNotNull]
public string FirstName { get; }

Y el constructor será este:

public Person([JetBrains.Annotations.NotNull] string firstName) {
  // pre not null check is implied by ConstructorParamNotNull
  if (firstName == null) throw new ArgumentNullException(nameof(firstName));

  FirstName = firstName;
  // + PostConstructor() if needed

  // post not null check implied by JetBrains.Annotations.NotNull on the property
  if (this.FirstName == null) throw new NullReferenceException(nameof(this.FirstName));
}

También hay algunas características más que esta.

Javier G.
fuente
Gracias. Lo acabo de probar y funcionó perfectamente. Marco esto como la respuesta aceptada, ya que aborda todos los problemas de la pregunta original.
Botond Balázs
16

Puede obtener un poco de mejora con una refactorización simple que puede aliviar el problema de escribir todas esas cercas. Primero, necesita este método de extensión:

internal static T ThrowIfNull<T>(this T o, string paramName) where T : class
{
    if (o == null)
        throw new ArgumentNullException(paramName);

    return o;
}

Entonces puedes escribir:

public class Person
{
    public string FullName { get; }
    public string NameAtBirth { get; }
    public string TaxId { get; }
    public PhoneNumber PhoneNumber { get; }
    public Address Address { get; }

    public Person(
        string fullName,
        string nameAtBirth,
        string taxId,
        PhoneNumber phoneNumber,
        Address address)
    {
        FullName = fullName.ThrowIfNull(nameof(fullName));
        NameAtBirth = nameAtBirth.ThrowIfNull(nameof(nameAtBirth));
        TaxId = taxId.ThrowIfNull(nameof(taxId));
        PhoneNumber = phoneNumber.ThrowIfNull(nameof(fullName));
        Address = address.ThrowIfNull(nameof(address));
    }
}

Devolver el parámetro original en el método de extensión crea una interfaz fluida, por lo que puede ampliar este concepto con otros métodos de extensión si lo desea, y encadenarlos todos juntos en su tarea.

Otras técnicas son más elegantes en concepto, pero progresivamente más complejas en ejecución, como decorar el parámetro con un [NotNull]atributo y usar Reflection de esta manera .

Dicho esto, es posible que no necesite todas estas pruebas, a menos que su clase sea parte de una API pública.

Robert Harvey
fuente
3
Es extraño que esto haya sido rechazado. Es muy similar al enfoque proporcionado por la respuesta más votada, excepto que no depende de Roslyn.
Robert Harvey
Lo que no me gusta de esto es que es vulnerable a modificaciones en el constructor.
jpmc26
@ jpmc26: ¿Qué significa eso?
Robert Harvey
1
Mientras leo su respuesta, sugiere que no pruebe el constructor , sino que cree un método común llamado en cada parámetro y lo pruebe. Si agrega un nuevo par de propiedad / argumento y olvida probar nullel argumento del constructor, no obtendrá una prueba fallida.
jpmc26
@ jpmc26: Naturalmente. Pero eso también es cierto si lo escribe de la manera convencional como se ilustra en el OP. Todo lo que hace el método común es mover el throwexterior del constructor; realmente no estás transfiriendo el control dentro del constructor a otro lugar. Es solo un método de utilidad, y una forma de hacer las cosas con fluidez , si así lo desea.
Robert Harvey
7

En el corto plazo, no hay mucho que pueda hacer sobre el tedio de escribir tales exámenes. Sin embargo, hay ayuda que viene con las expresiones de lanzamiento que se implementarán como parte de la próxima versión de C # (v7), probablemente en los próximos meses:

public class Person
{
    public string FullName { get; }
    public string NameAtBirth { get; }
    public string TaxId { get; }
    public PhoneNumber PhoneNumber { get; }
    public Address Address { get; }

    public Person(
        string fullName,
        string nameAtBirth,
        string taxId,
        PhoneNumber phoneNumber,
        Address address)
    {
        FullName = fullName ?? throw new ArgumentNullException(nameof(fullName));
        NameAtBirth = nameAtBirth ?? throw new ArgumentNullException(nameof(nameAtBirth));
        TaxId = taxId ?? throw new ArgumentNullException(nameof(taxId)); ;
        PhoneNumber = phoneNumber ?? throw new ArgumentNullException(nameof(phoneNumber)); ;
        Address = address ?? throw new ArgumentNullException(nameof(address)); ;
    }
}

Puede experimentar con expresiones de lanzamiento a través de la aplicación web Try Roslyn .

David Arno
fuente
Mucho más compacto, gracias. La mitad de las líneas. ¡Espero con ansias el C # 7!
Botond Balázs
@ BotondBalázs puedes probarlo en la vista previa de VS15 ahora mismo si lo deseas
user1306322
7

Me sorprende que nadie haya mencionado NullGuard.Fody todavía. Está disponible a través de NuGet y esos cheques nulos en la IL durante el tiempo de compilación.

Entonces su código de constructor simplemente sería

public Person(
    string fullName,
    string nameAtBirth,
    string taxId,
    PhoneNumber phoneNumber,
    Address address)
{
    FullName = fullName;
    NameAtBirth = nameAtBirth;
    TaxId = taxId;
    PhoneNumber = phoneNumber;
    Address = address;
}

y NullGuard agregará esas comprobaciones nulas para que usted las transforme en exactamente lo que escribió.

Sin embargo, tenga en cuenta que NullGuard es una opción de exclusión voluntaria , es decir, agregará esas comprobaciones nulas a cada método y argumento de constructor, getter y setter de propiedades e incluso comprobará los valores de retorno del método a menos que permita explícitamente un valor nulo con el [AllowNull]atributo.

Reiner romano
fuente
Gracias, esta parece una solución general muy buena. Aunque es muy desafortunado que modifique el comportamiento del lenguaje por defecto. Me gustaría mucho más si funcionara agregando un [NotNull]atributo en lugar de no permitir que nulo sea el predeterminado.
Botond Balázs
@ BotondBalázs: PostSharp tiene esto junto con otros atributos de validación de parámetros. Sin embargo, a diferencia de Fody, no es gratis. Además, el código generado llamará a las DLL de PostSharp, por lo que debe incluirlas con su producto. Fody solo ejecutará su código durante la compilación y no será necesario en tiempo de ejecución.
Roman Reiner el
@ BotondBalázs: También me pareció un poco extraño al principio que NullGuard se aplica por defecto, pero tiene sentido realmente. En cualquier base de código razonable, permitir nulo debería ser mucho menos común que verificar nulo. Si realmente desea permitir nulo en alguna parte, el enfoque de exclusión voluntaria lo obliga a ser muy exhaustivo con él.
Roman Reiner el
55
Sí, eso es razonable, pero viola el Principio de Menos Asombro . Si un nuevo programador no sabe que el proyecto usa NullGuard, ¡se llevará una gran sorpresa! Por cierto, tampoco entiendo el voto negativo, esta es una respuesta muy útil.
Botond Balázs
1
@ BotondBalázs / Roman, parece que NullGuard tiene un nuevo modo explícito que cambia la forma en que funciona de forma predeterminada
Kyle W
5

¿Vale la pena escribir todas estas pruebas?

No, probablemente no. ¿Cuál es la probabilidad de que lo arruines? ¿Cuál es la probabilidad de que algunas semánticas cambien de debajo de ti? ¿Cuál es el impacto si alguien lo arruina?

Si pasa mucho tiempo haciendo pruebas para algo que rara vez se romperá, y es una solución trivial si lo hiciera ... tal vez no valga la pena.

¿Sería una buena idea escribir un generador de código para tales clases para evitar escribir pruebas para cada una de ellas?

¿Tal vez? Ese tipo de cosas podría hacerse fácilmente con la reflexión. Algo a considerar es la generación de código para el código real, por lo que no tiene N clases que puedan tener un error humano. Prevención de errores> detección de errores.

Telastyn
fuente
Gracias. Tal vez no estaba lo suficientemente claro en mi pregunta, pero tenía la intención de escribir un generador que genere clases inmutables, no pruebas para ellos
Botond Balázs
2
También lo he visto varias veces en lugar de if...throwlo que tendrán ThrowHelper.ArgumentNull(myArg, "myArg"), de esa manera la lógica sigue ahí y no te molestan en la cobertura del código. Donde el ArgumentNullmétodo hará el if...throw.
Mateo
2

¿Vale la pena escribir todas estas pruebas?

No.
Porque estoy bastante seguro de que has probado esas propiedades a través de otras pruebas de lógica en las que se usan esas clases.

Por ejemplo, puede tener pruebas para la clase Factory que tengan una aserción basada en esas propiedades (instancia creada con la propiedad asignada correctamente Name, por ejemplo).

Si esas clases están expuestas a la API pública que utiliza un tercero / usuario final (@EJoshua gracias por darse cuenta), entonces las pruebas de lo esperado ArgumentNullExceptionpueden ser útiles.

Mientras espera C # 7, puede usar el método de extensión

public MyClass(string name)
{
    name.ThrowArgumentNullExceptionIfNull(nameof(name));
}

public static void ThrowArgumentNullExceptionIfNull(this object value, string paramName)
{
    if(value == null)
        throw new ArgumentNullException(paramName);
}

Para las pruebas, puede usar un método de prueba parametrizado que usa la reflexión para crear nullreferencias para cada parámetro y afirmar para la excepción esperada.

Fabio
fuente
1
Ese es un argumento convincente para no probar la inicialización de la propiedad.
Botond Balázs
Eso depende de cómo estés usando el código. Si el código es parte de una biblioteca pública, nunca debe confiar ciegamente en que otras personas sepan cómo funciona su biblioteca (especialmente si no tienen acceso al código fuente); sin embargo, si es algo que usa internamente para hacer que su propio código funcione, debe saber cómo usar su propio código, por lo que las comprobaciones tienen un propósito menor.
EJoshuaS - Restablece a Monica el
2

Siempre puedes escribir un método como el siguiente:

// Just to illustrate how to call this
private static void SomeMethod(string a, string b, string c, string d)
    {
        ValidateArguments(a, b, c, d);
        // ...
    }

    // This is the one to use as a utility function
    private static void ValidateArguments(params object[] args)
    {
        for (int i = 0; i < args.Length; i++)
        {
            if (args[i] == null)
            {
                StackTrace trace = new StackTrace();
                // Get the method that called us
                MethodBase info = trace.GetFrame(1).GetMethod();

                // Get information on the parameter that is null so we can add its name to the exception
                ParameterInfo param = info.GetParameters()[i];

                // Raise the exception on behalf of the caller
                throw new ArgumentNullException(param.Name);
            }
        }
    }

Como mínimo, eso le ahorrará algo de escritura si tiene varios métodos que requieren este tipo de validación. Por supuesto, esta solución supone que ninguno de los parámetros de su método puede serlo null, pero puede modificarlo para cambiarlo si así lo desea.

También puede extender esto para realizar otra validación específica de tipo. Por ejemplo, si tiene una regla de que las cadenas no pueden ser espacios en blanco o vacíos, puede agregar la siguiente condición:

// Note that we already know based on the previous condition that args[i] is not null
else if (args[i].GetType() == typeof(string))
            {
                string argCast = arg as string;

                if (!argCast.Trim().Any())
                {
                    ParameterInfo param = GetParameterInfo(i);

                    throw new ArgumentException(param.Name + " is empty or consists only of whitespace");
                }
            }

El método GetParameterInfo al que me refiero básicamente hace la reflexión (para que no tenga que seguir escribiendo lo mismo una y otra vez, lo que sería una violación del principio DRY ):

private static ParameterInfo GetParameterInfo(int index)
    {
        StackTrace trace = new StackTrace();

        // Note that we have to go 2 methods back to get the ValidateArguments method's caller
        MethodBase info = trace.GetFrame(2).GetMethod();

        // Get information on the parameter that is null so we can add its name to the exception
        ParameterInfo param = info.GetParameters()[index];

        return param;
    }
EJoshuaS - Restablece a Monica
fuente
1
¿Por qué se está rechazando esto? No está tan mal
Gaspa79
0

No sugiero lanzar excepciones del constructor. El problema es que tendrá dificultades para probar esto, ya que tendrá que pasar parámetros válidos, incluso si son irrelevantes para su prueba.

Por ejemplo: si desea probar si el tercer parámetro arroja una excepción, debe pasar el primero y el segundo correctamente. Entonces su prueba ya no está aislada. cuando las restricciones del primer y segundo parámetro cambian, este caso de prueba fallará incluso si prueba el tercer parámetro.

Sugiero usar la API de validación de Java y externalizar el proceso de validación. Sugiero tener cuatro responsabilidades (clases) involucradas:

  1. Un objeto de sugerencia con parámetros anotados de validación de Java con un método de validación que devuelve un conjunto de restricciones de restricción. Una ventaja es que puede pasar estos objetos sin que la afirmación sea válida. Puede retrasar la validación hasta que sea necesario sin intentar crear una instancia del objeto de dominio. El objeto de validación se puede usar en diferentes capas, ya que es un POJO sin conocimiento específico de la capa. Puede ser parte de tu API pública.

  2. Una fábrica para el objeto de dominio que es responsable de crear objetos válidos. Usted pasa el objeto de sugerencia y la fábrica llamará a la validación y creará el objeto de dominio si todo está bien.

  3. El objeto de dominio en sí mismo que debería ser en su mayoría inaccesible para la creación de instancias para otros desarrolladores. Para fines de prueba, sugiero tener esta clase en el alcance del paquete.

  4. Una interfaz de dominio público para ocultar el objeto de dominio concreto y dificultar el mal uso.

No sugiero verificar los valores nulos. Tan pronto como ingrese a la capa de dominio, se librará de pasar nulo o devolver nulo siempre que no tenga cadenas de objetos que tengan extremos o funciones de búsqueda de objetos individuales.

Otro punto: escribir la menor cantidad de código posible no es una métrica para la calidad del código. Piensa en 32k competiciones de juegos. Ese código es el más compacto pero también el más desordenado que puede tener, ya que solo se preocupa por cuestiones técnicas y no se preocupa por la semántica. Pero la semántica son los puntos que hacen que las cosas sean integrales.

oopexpert
fuente
1
Respetuosamente estoy en desacuerdo con tu último punto. No tener que escribir el mismo texto modelo para la 100ª vez no reducir la ayuda la posibilidad de cometer un error. Imagínese si esto fuera Java, no C #. Tendría que definir un campo de respaldo y un método getter para cada propiedad. El código sería completamente ilegible y lo importante sería oscurecido por la infraestructura torpe.
Botond Balázs