Precisamos validar o uso de todo o módulo ou apenas argumentos de métodos públicos?

9

Ouvi dizer que é recomendável validar argumentos de métodos públicos:

Motivação é compreensível. Se um módulo for usado de maneira errada, queremos lançar uma exceção imediatamente, em vez de qualquer comportamento imprevisível.

O que me incomoda é que argumentos errados não são o único erro que pode ser cometido ao usar um módulo. Aqui estão alguns cenários de erro em que precisamos adicionar a lógica de verificação se seguirmos as recomendações e não quisermos a escalação de erros:

  • Chamada recebida - argumentos inesperados
  • Chamada recebida - o módulo está em um estado incorreto
  • Chamada externa - resultados inesperados retornados
  • Chamada externa - efeitos colaterais inesperados (entrada dupla em um módulo de chamada, quebrando outros estados de dependências)

Eu tentei levar em conta todas essas condições e escrever um módulo simples com um método (desculpe, não caras do C #):

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;
    }
}

Agora é seguro =)

É bem assustador. Mas imagine como pode ser assustador em um módulo real com dezenas de métodos, estado complexo e muitas chamadas externas (oi, amantes de injeção de dependência!). Observe que se você está chamando um módulo cujo comportamento pode ser substituído (classe não selada em C #), está fazendo uma chamada externa e as consequências não são previsíveis no escopo do chamador.

Resumindo, qual é o caminho certo e por quê? Se você pode escolher entre as opções abaixo, responda a perguntas adicionais, por favor.

Verifique o uso de todo o módulo. Precisamos de testes de unidade? Existem exemplos desse código? A injeção de dependência deve ter uso limitado (uma vez que causará mais lógica de verificação)? Não é prático mover essas verificações para o tempo de depuração (não inclua na liberação)?

Verifique apenas argumentos. Pela minha experiência, a verificação de argumentos - especialmente a verificação nula - é a verificação menos eficaz, porque os erros de argumento raramente levam a erros complexos e a escalamentos de erros. Na maioria das vezes, você receberá um NullReferenceExceptionna próxima linha. Então, por que as verificações de argumentos são tão especiais?

Não verifique o uso do módulo. É uma opinião bastante impopular, você pode explicar por quê?

astef
fonte
As verificações devem ser feitas durante a atribuição de campo para garantir que os invariantes sejam mantidos.
Basilevs
@Basilevs Interessante ... É da ideologia do Code Contracts ou algo mais antigo? Você pode recomendar algo para ler (relacionado ao seu comentário)?
astef
É uma separação básica de preocupações. Todos os seus casos são abordados, enquanto a duplicação de código é mínima e as responsabilidades são bem definidas.
Basilevs
@Basilevs Portanto, não verifique o comportamento de outros módulos, mas verifique os invariantes do próprio estado. Parece razoável. Mas por que não vejo esse recibo simples nas perguntas relacionadas a verificações de argumentos?
astef 16/07/2015
Bem, algumas verificações comportamentais ainda são necessárias, mas devem ser realizadas apenas com valores realmente usados, não com os encaminhados para outro lugar. Por exemplo, você conta com a implementação da Lista para verificar erros OOB, em vez de verificar o índice no código do cliente. Geralmente, são falhas de estrutura de baixo nível e não precisam ser emitidas manualmente.
Basilevs

Respostas:

2

TL; DR: valida a mudança de estado, depende da [validade do] estado atual.

Abaixo, considero apenas as verificações habilitadas para lançamento. As asserções ativas somente para depuração são uma forma de documentação, útil à sua maneira e fora do escopo desta pergunta.

Considere os seguintes princípios:

  • Senso comum
  • Falhar rápido
  • SECO
  • SRP

Definições

  • Componente - uma unidade que fornece API
  • Cliente - usuário da API do componente

Estado mutável

Problema

Em linguagens imperativas, o sintoma do erro e sua causa podem ser separados por horas de trabalho pesado. A corrupção do estado pode se ocultar e sofrer mutações para resultar em falha inexplicável, pois a inspeção do estado atual não pode revelar o processo completo de corrupção e, portanto, a origem do erro.

Solução

Toda mudança de estado deve ser cuidadosamente elaborada e verificada. Uma maneira de lidar com o estado mutável é mantê-lo no mínimo. Isso é alcançado por:

  • sistema de tipos (declarações const e membros finais)
  • introdução de invariantes
  • verificar todas as alterações do estado do componente por meio de APIs públicas

Ao estender o estado de um componente, considere fazê-lo permitindo que o compilador imponha a imutabilidade de novos dados. Além disso, imponha todas as restrições de tempo de execução sensíveis, limitando os possíveis estados resultantes a um menor conjunto bem definido possível.

Exemplo

// 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;
    }
}

Repetição e coesão da responsabilidade

Problema

A verificação de pré-condições e pós-condições das operações leva à duplicação do código de verificação no cliente e no componente. A validação da chamada de componente geralmente força o cliente a assumir algumas responsabilidades do componente.

Solução

Confie no componente para executar a verificação do estado quando possível. Os componentes devem fornecer uma API que não exija verificação de uso especial (verificação de argumentos ou imposição da sequência de operações, por exemplo) para manter o estado do componente bem definido. Eles obrigam a verificar os argumentos de chamada da API, conforme necessário, relatam falhas pelos meios necessários e se esforçam para evitar a corrupção do estado.

Os clientes devem confiar nos componentes para verificar o uso de sua API. Não apenas a repetição é evitada, o cliente não depende mais de detalhes extras de implementação do componente. Considere a estrutura como um componente. Escreva apenas um código de verificação personalizado quando os invariantes do componente não forem suficientemente rígidos ou para encapsular a exceção dos componentes como detalhes da implementação.

Se uma operação não mudar de estado e não for coberta pelas verificações de mudança de estado, verifique todos os argumentos no nível mais profundo possível.

Exemplo

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;
   }
}

Responda

Quando os princípios descritos são aplicados ao exemplo em questão, obtemos:

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();            
    }
}

Sumário

O estado do cliente consiste em valores de campos próprios e partes do estado do componente que não são cobertas por seus próprios invariantes. A verificação deve ser feita apenas antes da alteração real do estado de um cliente.

Basilevs
fonte
1

Uma classe é responsável por seu próprio estado. Portanto, valide na medida em que mantém ou coloca as coisas em um estado aceitável.

Se um módulo for usado de maneira errada, queremos lançar uma exceção imediatamente, em vez de qualquer comportamento imprevisível.

Não, não lance uma exceção, mas entregue um comportamento previsível. O corolário da responsabilidade do Estado é tornar a classe / aplicação o mais tolerante possível. Por exemplo, passando nullpara aCollection.Add()? Apenas não adicione e continue. Você recebe nullinformações para criar um objeto? Crie um objeto nulo ou um objeto padrão. Acima, o doorjá é open? Então o que, continue. DoorFactoryargumento é nulo? Crie um novo. Quando crio um enum, sempre tenho um Undefinedmembro. I fazer uso liberal de Dictionarys e enumspara definir as coisas de forma explícita; e isso ajuda bastante a fornecer comportamentos previsíveis.

(oi, amantes de injeção de dependência!)

Sim, embora eu atravesse a sombra do vale dos parâmetros, não temerei argumentos. Para o anterior, eu também uso os parâmetros padrão e opcionais, tanto quanto possível.

Todas as opções acima permitem que o processamento interno continue. Em um aplicativo específico, tenho dezenas de métodos em várias classes, com apenas um único local onde uma exceção é lançada. Mesmo assim, não é por causa de argumentos nulos ou que não pude continuar processando, é porque o código acabou criando um objeto "não funcional" / "nulo".

editar

citando meu comentário na íntegra. Eu acho que o design não deve simplesmente "desistir" ao encontrar 'null'. Especialmente usando um objeto composto.

Estamos esquecendo os principais conceitos / premissas aqui - encapsulation& single responsibility. Praticamente não há verificação nula após a primeira camada de interação com o cliente. O código é robusto e tolerante . As classes são projetadas com estados padrão e, portanto, funcionam sem serem escritas, como se o código de interação fosse um lixo nocivo e desorganizado. Um pai composto não precisa acessar as camadas filho para avaliar a validade (e, implicitamente, verificar nulo em todos os cantos e recantos). Os pais sabem o que significa o estado padrão de uma criança

finalizar edição

radarbob
fonte
11
Não adicionar um elemento de coleção inválido é um comportamento muito imprevisível.
Basilevs
11
Se todas as interfaces forem projetadas de maneira tolerante, um dia, por causa de um erro banal, os programas despertarão acidentalmente e destruirão a humanidade.
astef
Estamos esquecendo os principais conceitos / premissas aqui - encapsulation& single responsibility. Praticamente não há nullverificação após a primeira camada de interação com o cliente. O código é <strike> tolerante </strike> robusto. As classes são projetadas com estados padrão e, portanto, funcionam sem serem gravadas, como se o código de interação fosse um lixo nocivo e desorganizado. Um pai composto não precisa acessar as camadas filho para avaliar a validade (e, implicitamente, verificar nulltodos os cantos e recantos). O pai sabe o que significa estado padrão de uma criança
radarbob