Validação de parâmetro do construtor em C # - Melhores práticas

34

Qual é a melhor prática para validação de parâmetro do construtor?

Suponha um bit simples de C #:

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

        // continue with normal construction
    }
}

Seria aceitável lançar uma exceção?

A alternativa que encontrei foi a pré-validação, 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
fonte
10
"aceitável lançar uma exceção?" Qual a alternativa?
S.Lott 23/02
10
@ S.Lott: Não faça nada. Defina um valor padrão. Imprima uma mesa no console / arquivo de log. Tocar um sino. Pisque uma luz.
FrustratedWithFormsDesigner
3
@FrustratedWithFormsDesigner: "Não faz nada?" Como será satisfeita a restrição "texto não vazio"? "Definir um valor padrão"? Como o valor do argumento de texto será usado? As outras idéias são boas, mas essas duas levariam a um objeto em um estado que viola as restrições dessa classe.
S.Lott 23/02
1
@ S.Lott Por medo de me repetir, eu estava aberto à possibilidade de haver alguma outra abordagem, que eu não conhecia. Acredito que não sei tudo, sei disso com certeza.
MPelletier
3
@ MPelletier, a validação antecipada dos parâmetros acabará violando DRY. (Não se repita) Como você precisará copiar essa pré-validação para qualquer código que tente instanciar essa classe.
CaffGeek 23/02

Respostas:

26

Costumo executar toda a minha validação no construtor. Isso é obrigatório, porque quase sempre crio objetos imutáveis. Para o seu caso específico, acho que isso é aceitável.

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

Se você estiver usando o .NET 4, poderá fazer isso. Obviamente, isso depende se você considera uma sequência que contém apenas espaço em branco inválida.

if (string.IsNullOrWhiteSpace(text))
    throw new ArgumentException("message", "text");
ChaosPandion
fonte
Não sei se o "caso específico" dele é específico o suficiente para dar conselhos específicos. Não temos idéia se faz sentido , neste contexto, lançar uma exceção ou simplesmente permitir que o objeto exista de qualquer maneira.
FrustratedWithFormsDesigner
@FrustratedWithFormsDesigner - Presumo que você tenha me votado negativamente por isso? É claro que você está certo de que estou extrapolando, mas claramente esse objeto teórico deve ser inválido se o texto de entrada estiver vazio. Você realmente gostaria de ligar Initpara receber algum tipo de código de erro quando uma exceção funcionar da mesma forma e forçar seu objeto a manter sempre um estado válido?
ChaosPandion
2
Eu costumo dividir esse primeiro caso em duas exceções separadas: uma que testa a nulidade e lança um ArgumentNullExceptione outra que testa vazio e lança um ArgumentException. Permite que o chamador seja mais exigente se desejar no tratamento de exceções.
Jesse C. Slicer
1
@ Jessie - usei essa técnica, mas ainda estou em dúvida sobre sua utilidade geral.
ChaosPandion
3
+1 para objetos imutáveis! Eu também os uso sempre que possível (eu pessoalmente acho quase sempre).
22

Muitas pessoas afirmam que os construtores não devem lançar exceções. O KyleG nesta página , por exemplo, faz exatamente isso. Honestamente, não consigo pensar em uma razão pela qual não.

No C ++, lançar uma exceção de um construtor é uma má idéia, pois deixa com memória alocada contendo um objeto não inicializado ao qual você não tem referência (ou seja, é um vazamento de memória clássico). Provavelmente é daí que vem o estigma - um monte de desenvolvedores de C ++ da velha escola gastaram metade do aprendizado de C # e apenas aplicaram o que sabiam do C ++ a ele. Por outro lado, no Objective-C, a Apple separou a etapa de alocação da etapa de inicialização, para que os construtores nesse idioma possam lançar exceções.

C # não pode vazar memória de uma chamada sem êxito para um construtor. Mesmo algumas classes na estrutura .NET lançam exceções em seus construtores.

Formiga
fonte
Você vai agitar algumas penas com seu comentário em C ++. :)
ChaosPandion
5
Ah, C ++ é uma ótima linguagem, mas uma das minhas irritações é a de pessoas que aprendem bem uma língua e depois escrevem assim o tempo todo . C # não é C ++.
Ant
10
Ainda pode ser perigoso lançar exceções de um ctor em C # porque o ctor pode ter alocado recursos não gerenciados que nunca serão descartados. E o finalizador precisa ser escrito para ser defensivo contra o cenário em que um objeto incompletamente construído é finalizado. Mas esses cenários são relativamente raros. Um próximo artigo no C # Dev Center abordará esses cenários e como codificá-los defensivamente, então observe esse espaço para obter detalhes.
Eric Lippert
6
Eh? lançar uma exceção do ctor é a única coisa que você pode fazer em c ++ se você não pode construir o objeto, e não há nenhuma razão que esta tem para causar um vazamento de memória (embora, obviamente, cabe a).
jk.
@jk: Hmm, você está certo! Embora pareça uma alternativa, é sinalizar objetos ruins como "zumbis", o que aparentemente o STL substitui: yosefk.com/c++fqa/exceptions.html#fqa-17.2 A solução do Objective-C é muito mais organizada.
Ant
12

Lance uma exceção IFF, a classe não pode ser colocada em um estado consistente com relação ao seu uso semântico. Caso contrário, não. NUNCA permita que um objeto exista em um estado inconsistente. Isso inclui não fornecer construtores completos (como ter um construtor vazio + inicializar () antes que seu objeto seja completamente construído) ... APENAS DIGA NÃO!

Em uma pitada, todo mundo faz isso. Fiz isso outro dia para um objeto usado de maneira muito restrita, dentro de um escopo restrito. Algum dia, no futuro, eu ou outra pessoa provavelmente pagaremos o preço por esse recibo na prática.

Devo observar que, por "construtor", quero dizer o que o cliente chama para criar o objeto. Isso poderia facilmente ser algo diferente de uma construção real chamada de "Construtor". Por exemplo, algo assim em C ++ não violaria o princípio 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>();
}

Como a única maneira de criar um funky_objecté chamar build_funkyo princípio de nunca permitir que um objeto inválido exista permaneça intacto, mesmo que o "Construtor" real não termine o trabalho.

Isso dá muito trabalho extra para ganho questionável (talvez até perda). Eu ainda preferiria a rota de exceção.

Edward Strange
fonte
9

Nesse caso, eu usaria o método de fábrica. Basicamente, defina sua classe apenas com construtores particulares e com um método de fábrica que retorna uma instância do seu objeto. Se os parâmetros iniciais forem inválidos, basta retornar nulo e solicitar que o código de chamada decida o que fazer.

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 lance exceções, a menos que as condições sejam excepcionais. Estou pensando que, neste caso, um valor vazio pode ser passado facilmente. Se for esse o caso, o uso desse padrão evitaria exceções e as penalidades de desempenho em que incorrem, mantendo o estado MyClass válido.

Donn Relacion
fonte
1
Eu não vejo a vantagem. Por que testar o resultado de uma fábrica como nulo é preferível a um try-catch em algum lugar que pode capturar a exceção do construtor? Sua abordagem parece ser "desconsiderar exceções, volte a verificar explicitamente o valor de retorno dos métodos para erros". Há muito tempo, aceitamos que as exceções geralmente são superiores a ter que testar explicitamente todos os métodos de valor-retorno de erros, portanto, seu conselho parece suspeito. Gostaria de ver uma justificativa para o motivo pelo qual você acha que a regra geral não se aplica aqui.
DW
nunca aceitamos que as exceções fossem superiores aos códigos de retorno; elas podem ser um sucesso enorme se lançadas com muita frequência. No entanto, lançar uma exceção de um construtor vazará memória, mesmo no .NET, se você alocou algo que não é gerenciado. Você precisa fazer muito trabalho no finalizador para compensar esse caso. De qualquer forma, a fábrica é uma boa ideia aqui, e TBH deve lançar uma exceção em vez de retornar um nulo. Supondo que você crie apenas algumas classes 'ruins', é preferível fazer o lançamento da fábrica.
Gbjbaanb
2
  • Um construtor não deve ter efeitos colaterais.
    • Qualquer coisa além da inicialização de campo privado deve ser vista como um efeito colateral.
    • Um construtor com efeitos colaterais quebra o Princípio de responsabilidade única (SRP) e contraria o espírito da programação orientada a objetos (OOP).
  • Um construtor deve ser leve e nunca deve falhar.
    • Por exemplo, eu sempre tremo quando vejo um bloco try-catch dentro de um construtor. Um construtor não deve lançar exceções ou erros de log.

Alguém poderia razoavelmente questionar essas diretrizes e dizer: "Mas eu não sigo essas regras, e meu código funciona bem!" A isso, eu respondia: "Bem, isso pode ser verdade, até que não seja".

  • Exceções e erros dentro de um construtor são muito inesperados. A menos que sejam instruídos a fazer isso, os futuros programadores não estarão inclinados a cercar essas chamadas de construtores com código defensivo.
  • Se algo falhar na produção, o rastreamento de pilha gerado poderá ser difícil de analisar. A parte superior do rastreamento da pilha pode apontar para a chamada do construtor, mas muitas coisas acontecem no construtor e pode não apontar para o LOC real que falhou.
    • Analisei muitos rastreamentos de pilha do .NET onde esse era o caso.
Jim G.
fonte
0

Depende do que o MyClass está fazendo. Se MyClass fosse realmente uma classe de repositório de dados e o texto do parâmetro fosse uma cadeia de conexão, a melhor prática seria lançar uma ArgumentException. No entanto, se MyClass for a classe StringBuilder (por exemplo), você poderá deixá-la em branco.

Portanto, depende de quão essencial é o texto do parâmetro para o método - o objeto faz sentido com um valor nulo ou em branco?

Steven Striga
fonte
2
Eu acho que a pergunta implica que a classe realmente exige que a string não esteja vazia. Esse não é o caso no seu exemplo StringBuilder.
Sergio Acosta
Então a resposta deve ser sim - é aceitável lançar uma exceção. Para evitar ter que tentar / capturar esse erro, você poderia usar um padrão de fábrica para lidar com a criação do objeto, o que incluiria um caso de string vazio.
Steven Striga
0

Minha preferência é definir um padrão, mas eu sei que o Java tem a biblioteca "Apache Commons" que faz algo assim, e acho que é uma boa idéia também. Não vejo um problema ao lançar uma exceção se o valor inválido colocar o objeto em um estado inutilizável; uma string não é um bom exemplo, mas e se fosse para o DI do pobre homem? Você não poderia operar se um valor de nullfosse passado no lugar de, digamos, uma ICustomerRepositoryinterface. Em uma situação como essa, lançar uma exceção é a maneira correta de lidar com as coisas, eu diria.

Wayne Molina
fonte
-1

Quando eu trabalhava em c ++, não costumava lançar exceções no construtor por causa do problema de vazamento de memória que isso pode levar, eu aprendi da maneira mais difícil. Qualquer pessoa que trabalha com c ++ sabe como pode ser difícil e problemático o vazamento de memória.

Mas se você estiver em c # / Java, não terá esse problema, porque o coletor de lixo coletará a memória. Se você estiver usando C #, acho preferível usar a propriedade de maneira agradável e consistente para garantir que as restrições de dados sejam garantidas.

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
fonte
-3

Lançar uma exceção será uma verdadeira dor de cabeça para os usuários de sua classe. Se a validação é um problema, geralmente faço da criação do objeto um processo de duas etapas.

1 - Crie a instância

2 - Chame um método Initialize com um resultado de retorno.

OU

Chame um método / função que crie a classe para você que possa fazer a validação.

OU

Tenho uma bandeira isValid, da qual eu particularmente não gosto, mas algumas pessoas gostam.

Dunk
fonte
3
@ Dunk, isso está errado.
CaffGeek 23/02
4
@ Chade, essa é a sua opinião, mas a minha opinião não está errada. É apenas diferente do seu. Acho o código try-catch muito mais difícil de ler e vi muito mais erros criados quando as pessoas usam o try-catch para manipulação trivial de erros do que os métodos mais tradicionais. Essa tem sido a minha experiência e não está errada. É o que é.
Dunk
5
O ponto é que, depois que eu chamo um construtor, se ele não cria porque uma entrada é inválida, isso é uma EXCEÇÃO. Os dados do aplicativo agora estão em um estado inconsistente / inválido se eu simplesmente criar o objeto e definir um sinalizador isValid = false. Ter que testar após a criação de uma classe se ela é válida é um design horrível e propenso a erros. E, você disse, tenha um construtor e chame initialize ... E se eu não chamar initialize? O que então? E se Initialize obtiver dados incorretos, posso lançar uma exceção agora? Sua classe não deve ser difícil de usar.
CaffGeek
5
@ Dunk, se você acha que as exceções são difíceis de usar, você as usa incorretamente. Simplificando, uma entrada ruim foi fornecida a um construtor. Isso é uma exceção. O aplicativo não deve continuar em execução, a menos que seja corrigido. Período. Se isso acontecer, você acaba com um problema pior: dados incorretos. Se você não sabe como lidar com a exceção, não sabe, você deixa a pilha de chamadas subir até que ela possa ser tratada, mesmo que isso signifique simplesmente registrar e interromper a execução. Simplificando, você não precisa lidar com exceções ou testá-las. Eles simplesmente funcionam.
CaffGeek
3
Desculpe, isso é um antipadrão demais a considerar.
MPelletier