Como defensivamente eu deveria programar? [fechadas]
https://stackoverflow.com/questions/1053215
-
20-08-2019 - |
italiano
english
français
española
中国
日本の
العربية
Deutsch
한국어
Português
RussianPergunta
Eu estava trabalhando com uma pequena rotina que é usado para criar uma conexão de banco de dados:
Antes
public DbConnection GetConnection(String connectionName)
{
ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];
DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);
DbConnection conn = factory.CreateConnection();
conn.ConnectionString = cs.ConnectionString;
conn.Open();
return conn;
}
Então eu comecei a olhar para a documentação .NET framework, para ver o que o documentado comportamento de várias coisas são, e ver se eu posso lidar com eles.
Por exemplo:
ConfigurationManager.ConnectionStrings...
O documentação diz que a chamada < strong> ConnectionStrings lança um ConfigurationErrorException se não poderia recuperar a coleção. Neste caso, não há nada que eu possa fazer para lidar com essa exceção, por isso vou deixá-lo ir.
Em seguida parte é a indexação real do ConnectionStrings para encontrar connectionName :
...ConnectionStrings[connectionName];
Neste caso, a documentação ConnectionStrings diz que a propriedade irá retornar nulo se o nome da conexão não pôde ser encontrado. i pode verificar que isso aconteça, e lançar uma exceção para deixar alguém alto que eles deram um connectionName inválido:
ConnectionStringSettings cs=
ConfigurationManager.ConnectionStrings[connectionName];
if (cs == null)
throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
i repetir o mesmo exercício com:
DbProviderFactory factory =
DbProviderFactories.GetFactory(cs.ProviderName);
O método GetFactory não tem nenhuma documentação sobre o que acontece se uma fábrica para o ProviderName especificado não pôde ser encontrado. Não é documentado para retornar null, mas eu ainda pode ficar na defensiva, e verificação para nulo:
DbProviderFactory factory =
DbProviderFactories.GetFactory(cs.ProviderName);
if (factory == null)
throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");
Em seguida é a construção do objeto DbConnection:
DbConnection conn = factory.CreateConnection()
Mais uma vez a documentação doesn 't dizer o que acontece se ele não poderia criar uma conexão, mas mais uma vez eu possa verificar se há um objeto nulo retorno:
DbConnection conn = factory.CreateConnection()
if (conn == null)
throw new Exception.Create("Connection factory did not return a connection object");
A seguir está definindo uma propriedade do objeto Connection:
conn.ConnectionString = cs.ConnectionString;
Os documentos não dizem o que acontece se ele não foi possível definir a seqüência de conexão. Será que lançar uma exceção? Será que ignorá-lo? Como com a maioria exceção, se houve um erro ao tentar definir o ConnectionString de uma conexão, não há nada que eu possa fazer para recuperar-se. Então eu vou fazer nada.
E, finalmente, abrir a conexão de banco de dados:
conn.Open();
O Abrir método de DbConnection é abstrata, por isso é até para qualquer provedor está descendo do DbConnection para decidir o que exceções eles jogam. Não há também nenhuma orientação em abstrato documentação métodos abertos sobre o que eu posso esperar que aconteça se houver um erro. Se não estava se conectando um erro, eu sei que não pode lidar com isso -. Eu vou ter que deixá-lo borbulhar onde o chamador pode mostrar alguma UI para o usuário, e deixá-los tentar novamente
Após
public DbConnection GetConnection(String connectionName)
{
//Get the connection string info from web.config
ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];
//documented to return null if it couldn't be found
if (cs == null)
throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
//Get the factory for the given provider (e.g. "System.Data.SqlClient")
DbProviderFactory factory = DbProviderFactories.GetFactory(cs.ProviderName);
//Undefined behaviour if GetFactory couldn't find a provider.
//Defensive test for null factory anyway
if (factory == null)
throw new Exception("Could not obtain factory for provider \""+cs.ProviderName+"\"");
//Have the factory give us the right connection object
DbConnection conn = factory.CreateConnection();
//Undefined behaviour if CreateConnection failed
//Defensive test for null connection anyway
if (conn == null)
throw new Exception("Could not obtain connection from factory");
//Knowing the connection string, open the connection
conn.ConnectionString = cs.ConnectionString;
conn.Open()
return conn;
}
Resumo
Assim, a minha função de quatro linhas, tornou-se 12 linhas, e exigiu 5 mintues de pesquisa de documentação. No final, i fiz captura um caso em que um método é deixada regressar até nula. Mas, na prática tudo que fiz foi converter uma exceção violação de acesso (se eu tentar chamar métodos em uma referência nula) em um InvalidArgumentException .
i também pegar dois casos possíveis onde poderia haver nulo objetos de retorno; mas mais uma vez eu só negociado uma exceção para outro.
No lado positivo, ele pegou dois problemas, e explicar o que aconteceu na mensagem de exceção, ao invés de coisas ruins happening na estrada (ou seja, o fanfarrão para aqui)
Mas vale a pena? É este um exagero? É este programação defensiva que deu errado?
Solução
manualmente a verificação de uma configuração e lançar uma exceção não é melhor do que apenas deixando o quadro lançar a exceção se a configuração está faltando é. Você está apenas duplicar cheques pré-condição que acontece dentro dos métodos de enquadramento de qualquer maneira, e isso faz você código detalhado sem nenhum benefício. (Na verdade, você pode ser remover informações jogando tudo como a classe base Exception. As exceções lançadas pela estrutura são tipicamente mais específico.)
Edit: Esta resposta parece ser um tanto controverso, por isso, um pouco de elaboração: meios de programação defensiva "preparar para o inesperado" (ou "ser paranóico") e uma das maneiras de fazer isso é fazer com que lotes de cheques pré-condição . Em muitos casos, isso é uma boa prática, no entanto, como com todas as práticas de custo deve ser pesado contra os benefícios.
Por exemplo, não fornece qualquer benefício para lançar uma exceção "Não foi possível obter conexão de fábrica", uma vez que não diz nada sobre por não foi possível obter o provedor - eo muito próxima linha vai lançar uma qualquer maneira exceção se o provedor é nulo. Assim, o custo do cheque pré-condição (em tempo de desenvolvimento e complexidade do código) não se justifica.
Por outro lado, a verificação para verificar se a configuração seqüência de conexão está presente pode ser justificada, uma vez que a exceção pode ajudar a contar o desenvolvedor como resolver o problema. O nulo exceção que você terá na próxima linha de qualquer maneira não diz o nome da cadeia de conexão que está faltando, então o cheque pré-condição não fornecer algum valor. Se o seu código é parte de um componente, por exemplo, o valor é muito grande, uma vez que o usuário do componente pode não saber quais configurações do componente requer.
Uma interpretação diferente de programação defensiva é que você não deve apenas detectar condições de erro, você também deve tentar recuperar de qualquer erro ou exceção que possa ocorrer. Eu não acredito que esta é uma boa idéia em geral.
Basicamente, você só deve lidar com exceções que você pode fazer algo sobre. Exceções que você não pode recuperar de qualquer maneira, deve apenas ser passado para cima para o manipulador de nível superior. Em uma aplicação web o manipulador de nível superior, provavelmente, apenas mostra uma página de erro genérico. Mas não há realmente muito a fazer na maioria dos casos, se o banco de dados está off-line ou alguma configuração importante está faltando.
Alguns casos em que esse tipo de programação defensiva faz sentido, é se você aceitar a entrada do usuário, e que a entrada pode levar a erros. Se, por exemplo, o usuário fornece uma URL como entrada, e as tentativas de aplicação para buscar algo de que URL, então é muito importante que você verifique se os olhares URL correta, e você lidar com qualquer exceção que pode resultar do pedido. Isso permite que você fornecer um feedback valioso para o usuário.
Outras dicas
Bem, depende de quem é seu público.
Se você estiver escrevendo código da biblioteca que você espera para ser usado por um monte de outras pessoas, que não vai estar falando com você sobre como usá-lo, então não é um exagero. Eles vão apreciar o seu esforço.
(Dito isto, se você está fazendo isso, eu sugiro que você definir melhores exceções que apenas System.Exception, para tornar mais fácil para as pessoas que querem pegar algumas de suas exceções, mas não outros.)
Mas se você está indo só para usá-lo você mesmo (ou você e seu amigo), então obviamente é um exagero, e, provavelmente, fere-lo no final, tornando o seu código menos legível.
Eu gostaria de poder começar a minha equipe para um código como este. A maioria das pessoas ainda não entendo o ponto de programação defensiva. O melhor que eles fazem é para embrulhar o método inteiro em uma declaração try catch e deixe todas as exceções ser tratado pelo bloco de exceção genérica!
Tiremos o chapéu para você Ian. Eu posso entender o seu dilema. Já passei o mesmo a mim mesmo. Mas o que você fez provavelmente ajudou algum desenvolvedor de várias horas de contusão teclado.
Lembre-se, quando você estiver usando um .NET Framework API, o que você espera dele? O que parece natural? Faça o mesmo com o seu código.
Eu sei que leva tempo. Mas, em seguida, a qualidade tem um custo.
PS: Você realmente não tem que lidar com todos os erros e lançar uma exceção personalizada. Lembre-se que seu método irá ser utilizado por outros desenvolvedores. Eles devem ser capazes de descobrir próprios exceções quadro comum. Que não vale a pena.
O "antes" exemplo tem a distinção de ser clara e concisa.
Se algo está errado, uma exceção será lançada, eventualmente, pela estrutura. Se você não pode fazer nada sobre a exceção, assim como você pode deixá-lo propagar-se a pilha de chamadas.
Há momentos, no entanto, quando uma exceção é lançada profundamente dentro da estrutura que realmente não lançar luz sobre o que o problema é real. Se o seu problema é que você não tem uma seqüência de conexão válida, mas o quadro lança uma exceção como "uso inválido de nulo", então às vezes é melhor para capturar a exceção e relançar-lo com uma mensagem que é mais significativo.
Eu faço seleção para nula objetos muito, desde que eu preciso um objeto real para operar em, e se o objeto é esvaziar a exceção que é lançada será oblíqua, para dizer o mínimo. Mas eu só buscar por objetos nulos se eu sei que é o que vai ocorrer. Algumas fábricas de objetos não retornam objetos nulos; eles lançar uma exceção em vez disso, e verificação de nulo será inútil nesses casos.
Eu não acho que eu iria escrever nada disso lógica verificação nulo de referência -., Pelo menos, não da maneira que você fez isso
Os meus programas que recebem as definições de configuração a partir do arquivo de configuração do aplicativo verificar todas essas configurações na inicialização. Eu costumo criar uma classe estática para conter as configurações e fazer referência a propriedades dessa classe (e não o ConfigurationManager) em outras partes do aplicativo. Há duas razões para isso.
Em primeiro lugar, se o aplicativo não está configurado corretamente, não é indo para o trabalho. Eu prefiro saber isso no momento em que o programa lê o arquivo de configuração do que em algum momento no futuro, quando eu tento criar uma conexão de banco de dados.
Em segundo lugar, verificar a validade de configuração não deve ser realmente a preocupação dos objetos que dependem da configuração. Não há nenhum sentido em fazer-se inserir verificações em todos os lugares em todo o código se você já tiver realizado os cheques na frente. (Há exceções a esta, certamente - por exemplo, de longa duração aplicações onde você precisa ser capaz de alterar a configuração enquanto o programa está em execução e têm essas alterações refletidas no comportamento do programa, em programas como este você vai precisar ir ao ConfigurationManager sempre que precisar de uma definição.)
Eu não faria a verificação nulo referência sobre as chamadas GetFactory e CreateConnection, tampouco. Como você escrever um caso de teste para exercer esse código? Você não pode, porque você não sabe como fazer esses métodos retornam null - você nem sequer sabe que é possível para fazer esses métodos retornam nulo. Assim você substituiu um problema - seu programa pode jogar um NullReferenceException sob condições que você não entende - com outro, um mais significativo: sob essas mesmas condições misteriosas, o programa irá executar o código que você não tenha testado
A minha regra de polegares é:
não pegar, se a mensagem do exceção lançada é relevante para o chamador.
Assim, NullReferenceException não tem uma mensagem relevante, gostaria de verificar se é nulo e lançar uma exceção com uma mensagem melhor. ConfigurationErrorException é relevante para que eu não pegá-lo.
A única exceção é se o "contrato" de GetConnection não recupera a seqüência de conexão, necessariamente, em um arquivo de configuração.
Se for o GetConnection caso deve ter um contrato com uma exceção personalizada que dizer que a conexão não pôde ser recuperada, então você pode envolver ConfigurationErrorException em sua exceção personalizada.
A outra solução é para especificar que GetConnection não pode jogar (mas pode retornar null), então você adicionar um "ExceptionHandler" para sua classe.
A documentação do método está faltando. ; -)
Cada método tem alguns parâmetros de entrada e de saída definidas e um comportamento resultante definido. No seu caso algo como: ". Returns conexão aberta válida em caso de sucesso, retorna outra nula (ou lança uma XXXException, como você gosta) Manter este comportamento em mente, agora você pode decidir como defensivo você deve programar
.-
Se o seu método deve expor informações detalhadas porquê eo que falhou, então fazê-lo como você fez e verificar e pegar tudo e todos e retornar as informações adequadas.
-
Mas, se você está apenas interessado em um DbConnection aberto ou apenas nulo (ou algum usuário exceção definida) em caso de falha, em seguida, basta embrulhar tudo em um try / catch e retorno nulo (ou alguma exceção) em caso de erro, e o objeto mais.
Então, eu diria, depende do comportamento do método e saída esperada.
Em geral, as exceções específicas do banco de dados deve ser capturado e re-lançada como algo mais geral, como (hipotético) de exceção DataAccessFailure. código de nível superior na maioria dos casos não precisa saber que você está lendo os dados do banco de dados.
Outra razão para interceptar esses erros rapidamente é que eles muitas vezes incluem alguns detalhes de banco de dados em suas mensagens, como "No such tabela: ACCOUNTS_BLOCKED" ou "chave de usuário inválido: 234234". Se este se propaga para o usuário final, é ruim de várias maneiras:
- confuso.
- violação de segurança potencial.
- embaraçoso para a imagem da sua empresa (imagine um cliente lendo uma mensagem de erro com a gramática bruto).
Eu teria codificado-lo exatamente como sua primeira tentativa.
No entanto, o usuário de que a função iria proteger o objeto de conexão com um bloco usando.
eu realmente não sei como traduzir exceções como suas outras versões fazer o que faz com que seja muito difícil de descobrir porque ele quebrou (Banco de Dados para baixo? Não tem permissão para ler o arquivo de configuração, etc?).
A versão alterada não acrescenta muito valor, desde que o aplicativo tem um manipulador de AppDomain.UnexpectedException que despeja a cadeia exception.InnerException e todos os rastreamentos de pilha em um arquivo de log de algum tipo (ou melhor ainda, capturas de um minidump) e, em seguida, chamada Environment.FailFast.
A partir dessa informação, será relativamente simples de identificar o que deu errado, sem a necessidade de complicar o código de método com verificação de erro adicional.
Note que é preferível AppDomain.UnexpectedException alça e Environment.FailFast chamada em vez de ter um try/catch (Exception x) de nível superior porque com este último, a causa original do problema provavelmente será obscurecido por outras excepções.
Isso porque, se você capturar uma exceção, todos os blocos aberta finally irá executar, e provavelmente irá lançar mais exceções, que irá esconder a exceção original (ou pior, eles vão excluir arquivos em uma tentativa de reverter algum estado, possivelmente o arquivos errado, talvez até mesmo arquivos importantes). Você nunca deve capturar uma exceção que indica um estado programa inválido que você não sabe como lidar com - mesmo em um nível main bloco de função try/catch superior. Manipulação AppDomain.UnexpectedException e chamando Environment.FailFast é uma chaleira de peixes diferente (e mais desejável), porque ele pára blocos finally de correr, e se você está tentando parar o seu programa e log algumas informações úteis sem fazer mais danos, você definitivamente não fazer deseja executar seus blocos finally.
Não se esqueça de verificar se há OutOfMemoryExceptions ... você sabe, ele pode acontecer.
mudanças de Iain olhar sensível para mim.
Se eu estou usando um sistema e eu usá-lo indevidamente, eu quero o máximo de informações sobre o uso indevido. Por exemplo. se eu esquecer de inserir alguns valores em uma configuração antes de chamar um método, eu quero um InvalidOperationException com uma mensagem detalhando o meu erro, não um KeyNotFoundException / NullReferenceException.
É tudo sobre contexto IMO. Eu vi algumas mensagens de exceção bastante impenetráveis ??no meu tempo, mas outras vezes a exceção padrão que vem do quadro é perfeitamente bem.
Em geral, eu acho que é melhor errar do lado da cautela, especialmente quando você está escrevendo algo que é muito utilizada por outras pessoas ou normalmente no fundo do gráfico de chamada onde o erro é mais difícil de diagnosticar.
Eu sempre tento lembrar que, como um desenvolvedor de um pedaço de código ou sistema, eu estou em uma posição melhor para diagnosticar falhas do que alguém que está apenas usando-o. Às vezes é o caso que algumas linhas de código de verificação + uma mensagem exceção personalizada pode cumulativamente economizar horas de depuração tempo (e também fazer a sua própria vida mais fácil, pois você não se puxado em torno de outra pessoa máquina para depurar seu problema).
Em meus olhos, o "depois" amostra não é realmente defensiva. Porque defensiva seria a de verificar as partes sob seu controle, o que seria o connectionName argumento (verificação para nulo ou vazio, e lançar um ArgumentNullException).
Por que não dividir o método que você tem depois de ter adicionado toda a programação defensiva? Você tem um monte de blocos lógicos distintas que justificam métodos distintos. Por quê? Porque então você encapsular a lógica que pertence juntos, e seu método pública resultando apenas fios esses blocos no caminho certo.
Algo como isto (editado no editor de SO, então nenhuma verificação de sintaxe / compilador não pôde compilar; -).)
private string GetConnectionString(String connectionName)
{
//Get the connection string info from web.config
ConnectionStringSettings cs= ConfigurationManager.ConnectionStrings[connectionName];
//documented to return null if it couldn't be found
if (cs == null)
throw new ArgumentException("Could not find connection string \""+connectionName+"\"");
return cs;
}
private DbProviderFactory GetFactory(String ProviderName)
{
//Get the factory for the given provider (e.g. "System.Data.SqlClient")
DbProviderFactory factory = DbProviderFactories.GetFactory(ProviderName);
//Undefined behaviour if GetFactory couldn't find a provider.
//Defensive test for null factory anyway
if (factory == null)
throw new Exception("Could not obtain factory for provider \""+ProviderName+"\"");
return factory;
}
public DbConnection GetConnection(String connectionName)
{
//Get the connection string info from web.config
ConnectionStringSettings cs = GetConnectionString(connectionName);
//Get the factory for the given provider (e.g. "System.Data.SqlClient")
DbProviderFactory factory = GetFactory(cs.ProviderName);
//Have the factory give us the right connection object
DbConnection conn = factory.CreateConnection();
//Undefined behaviour if CreateConnection failed
//Defensive test for null connection anyway
if (conn == null)
throw new Exception("Could not obtain connection from factory");
//Knowing the connection string, open the connection
conn.ConnectionString = cs.ConnectionString;
conn.Open()
return conn;
}
PS:. Este não é um refactor completa, só fez os dois primeiros blocos
