¿Necesitamos validar el uso del módulo completo o solo argumentos de métodos públicos?

9

He oído que se recomienda validar argumentos de métodos públicos:

La motivación es comprensible. Si un módulo se usará de manera incorrecta, queremos lanzar una excepción de inmediato en lugar de cualquier comportamiento impredecible.

Lo que me molesta es que los argumentos incorrectos no son el único error que se puede cometer al usar un módulo. Aquí hay algunos escenarios de error en los que necesitamos agregar lógica de verificación si seguimos las recomendaciones y no queremos una escalada de errores:

  • Llamada entrante - argumentos inesperados
  • Llamada entrante: el módulo está en un estado incorrecto
  • Llamada externa: resultados inesperados devueltos
  • Llamada externa - efectos secundarios inesperados (doble entrada a un módulo de llamada, rompiendo otros estados de dependencias)

Intenté tener en cuenta todas estas condiciones y escribir un módulo simple con un método (lo siento, no C-chicos):

public sealed class Room
{
    private readonly IDoorFactory _doorFactory;
    private bool _entered;
    private IDoor _door;
    public Room(IDoorFactory doorFactory)
    {
        if (doorFactory == null)
            throw new ArgumentNullException("doorFactory");
        _doorFactory = doorFactory;
    }
    public void Open()
    {
        if (_door != null)
            throw new InvalidOperationException("Room is already opened");
        if (_entered)
            throw new InvalidOperationException("Double entry is not allowed");
        _entered = true;
        _door = _doorFactory.Create();
        if (_door == null)
            throw new IncompatibleDependencyException("doorFactory");
        _door.Open();
        _entered = false;
    }
}

Ahora es seguro =)

Es bastante espeluznante. Pero imagine lo espeluznante que puede ser en un módulo real con docenas de métodos, estado complejo y muchas llamadas externas (¡hola, amantes de la inyección de dependencia!). Tenga en cuenta que si está llamando a un módulo cuyo comportamiento se puede anular (clase no sellada en C #), entonces está haciendo una llamada externa y las consecuencias no son predecibles en el alcance de la persona que llama.

En resumen, ¿cuál es la forma correcta y por qué? Si puede elegir entre las opciones a continuación, responda preguntas adicionales, por favor.

Verifique el uso completo del módulo. ¿Necesitamos pruebas unitarias? ¿Hay ejemplos de tal código? ¿Debe la inyección de dependencia tener un uso limitado (ya que provocará más lógica de comprobación)? ¿No es práctico mover esos controles al tiempo de depuración (no incluir en la versión)?

Marque solo argumentos. Según mi experiencia, la verificación de argumentos, especialmente la verificación nula, es la verificación menos efectiva, porque el error de argumento rara vez conduce a errores complejos y escalas de errores. La mayoría de las veces obtendrá un NullReferenceExceptionen la siguiente línea. Entonces, ¿por qué las verificaciones de argumentos son tan especiales?

No verifique el uso del módulo. Es una opinión bastante impopular, ¿puedes explicar por qué?

astef
fuente
Se deben realizar verificaciones durante la asignación de campo para garantizar que se mantengan los invariantes.
Basilevs
@Basilevs Interesante ... ¿Es de la ideología de Code Contracts o algo más antiguo? ¿Me puede recomendar algo para leer (relacionado con su comentario)?
astef
Es una separación básica de preocupaciones. Todos sus casos están cubiertos, mientras que la duplicación de código es mínima y las responsabilidades están bien definidas.
Basilevs
@Basilevs Entonces, no verifique el comportamiento de otros módulos en absoluto, sino verifique los invariantes propios del estado. Suena razonable. Pero, ¿por qué no veo este simple recibo en las preguntas relacionadas sobre la verificación de argumentos?
astef
Bueno, todavía se necesitan algunas comprobaciones de comportamiento, pero solo deben realizarse en valores realmente utilizados, no en los que se envían a otro lugar. Por ejemplo, confía en la implementación de la Lista para verificar los errores OOB, en lugar de verificar el índice en el código del cliente. Por lo general, son fallas de marco de bajo nivel y no requieren ser emitidas manualmente.
Basilevs

Respuestas:

2

TL; DR: Validar el cambio de estado, confiar en [validez del] estado actual.

A continuación solo considero las verificaciones habilitadas para el lanzamiento. Las afirmaciones activas solo de depuración son una forma de documentación, que es útil a su manera y está fuera del alcance de esta pregunta.

Considere los siguientes principios:

  • Sentido común
  • Fallar rapido
  • SECO
  • SRP

Definiciones

  • Componente: una unidad que proporciona API
  • Cliente: usuario de la API del componente

Estado mutable

Problema

En idiomas imperativos, el síntoma de error y su causa pueden estar separados por horas de trabajo pesado. La corrupción estatal puede ocultarse y mutar para dar como resultado una falla inexplicable, ya que la inspección del estado actual no puede revelar el proceso completo de corrupción y, por lo tanto, el origen del error.

Solución

Cada cambio de estado debe ser cuidadosamente elaborado y verificado. Una forma de lidiar con el estado mutable es mantenerlo al mínimo. Esto se logra mediante:

  • sistema de tipos (const y declaraciones de miembros finales)
  • introduciendo invariantes
  • verificar cada cambio de estado del componente a través de API públicas

Al extender el estado de un componente, considere hacerlo permitiendo que el compilador imponga la inmutabilidad de los nuevos datos. Además, aplique cada restricción de tiempo de ejecución razonable, limitando los posibles estados resultantes a un conjunto bien definido lo más pequeño posible.

Ejemplo

// Wrong
class Natural {
    private int number;
    public Natural(int number) {
        this.number = number;
    }
    public int getInt() {
      if (number < 1)
          throw new InvalidOperationException();
      return number;
    }
}

// Right
class Natural {
    private readonly int number;
    /**
     * @param number - positive number
     */
    public Natural(int number) {
      // Going to modify state, verification is required
      if (number < 1)
        throw new ArgumentException("Natural number should be  positive: " + number);
      this.number = number;
    }
    public int getInt() {
      // State is guaranteed by construction and compiler
      return number;
    }
}

Repetición y cohesión de responsabilidad

Problema

La verificación de las condiciones previas y las condiciones posteriores de las operaciones conduce a la duplicación del código de verificación tanto en el cliente como en el componente. La validación de la invocación de componentes a menudo obliga al cliente a asumir algunas de las responsabilidades del componente.

Solución

Confíe en el componente para realizar la verificación de estado cuando sea posible. Los componentes deben proporcionar una API que no requiera una verificación de uso especial (verificación de argumentos o aplicación de secuencia de operación, por ejemplo) para mantener el estado del componente bien definido. Obligan a verificar los argumentos de invocación de API según sea necesario, informan fallas por los medios necesarios y se esfuerzan por evitar la corrupción de su estado.

Los clientes deben confiar en los componentes para verificar el uso de su API. No solo se evita la repetición, el cliente ya no depende de detalles adicionales de implementación del componente. Considere el marco como un componente. Solo escriba código de verificación personalizado cuando los invariantes de los componentes no sean lo suficientemente estrictos o para encapsular la excepción de los componentes como detalle de implementación.

Si una operación no cambia de estado y no está cubierta por las verificaciones de cambio de estado, verifique cada argumento en el nivel más profundo posible.

Ejemplo

class Store {
  private readonly List<int> slots = new List<int>();
  public void putToSlot(int slot, int data) {
    if (slot < 0 || slot >= slots.Count) // Unnecessary, validated by List, only needed for custom error message
      throw new ArgumentException("data");
    slots[slot] = data;
  }
}

class Natural {
   int _number;
   public Natural(int number) {
       if (number < 1)
          number = 1;  //Wrong: client can't rely on argument verification, additional state uncertainity is introduced.  Right: throw new ArgumentException(number);
       _number = number;
   }
}

Responder

Cuando los principios descritos se aplican al ejemplo en cuestión, obtenemos:

public sealed class Room
{
    private bool _entered = false;
    // Do not use lazy instantiation if not absolutely necessary, this introduces additional mutable state
    private readonly IDoor _door;
    public Room(IDoorFactory doorFactory)
    {
        // Rely on system null check
        IDoor door = _doorFactory.Create();
        // Modifying own state, verification is required
        if (door == null)
           throw new ArgumentNullException("Door");
        _door = door;
    }
    public void Enter()
    {
        // Room invariants do not guarantee _entered value. Door state is indirectly a part of our state. Verification is required to prevent second door state change below.
        if (_entered)
           throw new InvalidOperationException("Double entry is not allowed");
        _entered = true;     
        // rely on immutability for _door field to be non-null
        // rely on door implementation to control resulting door state       
        _door.Open();            
    }
}

Resumen

El estado del cliente consta de valores de campos propios y partes del estado del componente que no están cubiertos por sus propios invariantes. La verificación solo debe hacerse antes del cambio de estado real de un cliente.

Basilevs
fuente
1

Una clase es responsable de su propio estado. Por lo tanto, valide en la medida en que mantenga o ponga las cosas en un estado aceptable.

Si un módulo se utilizará de manera incorrecta, queremos lanzar una excepción de inmediato en lugar de cualquier comportamiento impredecible.

No, no arrojes una excepción, en su lugar, ofrece un comportamiento predecible. El corolario de la responsabilidad estatal es hacer que la clase / aplicación sea tan tolerante como práctica. Por ejemplo, pasando nulla aCollection.Add()? Simplemente no agregue y continúe. ¿Obtiene nullinformación para crear un objeto? Cree un objeto nulo o un objeto predeterminado. Arriba, el doorya es open? Y qué, sigue adelante. DoorFactoryargumento es nulo? Crea uno nuevo. Cuando creo un enumsiempre tengo un Undefinedmiembro. Hago un uso liberal de Dictionarysy enumspara definir las cosas explícitamente; y esto contribuye en gran medida a ofrecer un comportamiento predecible.

(hola, amantes de la inyección de dependencia!)

Sí, aunque camino a través de la sombra del valle de parámetros, no temeré discusiones. Para lo anterior, también uso los parámetros predeterminados y opcionales tanto como sea posible.

Todo lo anterior permite que el procesamiento interno continúe. En una aplicación particular, tengo docenas de métodos en varias clases con un solo lugar donde se lanza una excepción. Incluso entonces, no es por argumentos nulos o porque no pude continuar procesando, es porque el código terminó creando un objeto "no funcional" / "nulo".

editar

citando mi comentario en su totalidad. Creo que el diseño no debería simplemente "darse por vencido" al encontrar 'nulo'. Especialmente usando un objeto compuesto.

Aquí estamos olvidando conceptos / supuestos clave - encapsulation& single responsibility. Prácticamente no hay verificación nula después de la primera capa que interactúa con el cliente. El código es tolerante robusto. Las clases están diseñadas con estados predeterminados y, por lo tanto, funcionan sin estar escritas, como si el código de interacción estuviera lleno de errores, basura no autorizada. Un padre compuesto no tiene que alcanzar las capas secundarias para evaluar la validez (y, por implicación, verificar si hay nulos en todos los rincones). El padre sabe lo que significa el estado predeterminado de un niño

fin editar

radarbob
fuente
1
No agregar un elemento de colección no válido es un comportamiento muy impredecible.
Basilevs
1
Si todas las interfaces se diseñan de una manera tan tolerante, un día, debido a un error banal, los programas accidentalmente despertarán y destruirán a la humanidad.
astef
Aquí estamos olvidando conceptos / supuestos clave - encapsulation& single responsibility. Prácticamente no hay nullverificación después de la primera capa que interactúa con el cliente. El código es <strike> tolerante </strike> robusto. Las clases están diseñadas con estados predeterminados y, por lo tanto, funcionan sin estar escritas como si el código de interacción estuviera lleno de errores, basura no autorizada. Un padre compuesto no tiene que alcanzar las capas secundarias para evaluar la validez (y por implicación, verificar nullen todos los rincones y grietas). El padre sabe lo que significa el estado predeterminado de un niño
radarbob