Gostaria de saber se existem prós e contras contra esse estilo:
private void LoadMaterial(string name)
{
if (_Materials.ContainsKey(name))
{
throw new ArgumentException("The material named " + name + " has already been loaded.");
}
_Materials.Add(
name,
Resources.Load(string.Format("Materials/{0}", name)) as Material
);
}
Esse método deve, para cada um name
, ser executado apenas uma vez. _Materials.Add()
emitirá uma exceção se for chamado várias vezes para o mesmo name
. Minha guarda, como resultado, é completamente redundante ou há alguns benefícios menos óbvios?
Isso é C #, Unity, se alguém estiver interessado.
exceptions
assíncrono
fonte
fonte
_Materials.Add
lançar uma exceção?string.Format
concatenação de cadeia de caracteres para criar a mensagem de exceção. 2) use apenasas
se você espera que o elenco falhe e verifique o resultadonull
. Se você espera obter sempre umMaterial
, use um elenco como(Material)Resources.Load(...)
. Uma exceção de conversão clara é mais fácil de depurar do que uma exceção de referência nula que ocorre posteriormente.LoadMaterialIfNotLoaded
ouReloadMaterial
esse nome pode usar melhorias).Respostas:
O benefício é que sua exceção "personalizada" possui uma mensagem de erro significativa para quem chama essa função sem saber como ela é implementada (que no futuro pode ser você!).
É verdade que, nesse caso, eles provavelmente poderiam adivinhar o que significava a exceção "padrão", mas você ainda está deixando claro que eles violaram seu contrato, em vez de tropeçarem em algum bug estranho no seu código.
fonte
_Materials.Add
não seja o erro que você deseja transmitir ao chamador, acho que esse método de rotular novamente o erro é ineficiente. Para cada sucesso chamada deLoadMaterial
(operação normal), você vai ter feito o mesmo teste de sanidade duas vezes , emLoadMaterial
e depois novamente em_Materials.Add
. Se mais camadas forem agrupadas, cada uma empregando o mesmo estilo, você poderá ter muitas mais vezes o mesmo teste._Materials.Add
incondicionalmente, depois pegue um erro em potencial e, no manipulador, lance um erro diferente. O trabalho extra agora é feito apenas em casos errôneos, o caminho de execução excepcional em que você realmente não se preocupa com a eficiência.Eu concordo com a resposta do Ixrec . No entanto, você pode considerar uma terceira alternativa: tornar a função idempotente. Em outras palavras, retorne cedo, em vez de jogar um
ArgumentException
. Isso geralmente é preferível se você for forçado a verificar se ele já foi carregado antes de ligarLoadMaterial
todas as vezes. Quanto menos condições prévias você tiver, menor será a carga cognitiva no programador.Lançar uma exceção seria preferível se for realmente um erro do programador, onde deve ser óbvio e conhecido em tempo de compilação se o material já foi carregado, sem ter que verificar em tempo de execução antes de chamar.
fonte
LoadMaterial
tem as seguintes restrições: "Depois de chamá-lo, o material é sempre carregado e o número de materiais carregados não diminui". Lançar uma exceção para uma terceira restrição "O material não pode ser adicionado duas vezes" parece bobagem e contra-intuitivo para mim. Tornar a função idempotente reduz a dependência do código na ordem de execução e no estado do aplicativo. Parece uma vitória dupla para mim.A pergunta fundamental que você deve fazer é: qual a interface para sua função? Em particular, a condição é
_Materials.ContainsKey(name)
uma pré-condição da função?Se é não uma condição prévia, a função deve fornecer um resultado bem definido para todos os vales possíveis de
name
. Nesse caso, a exceção lançada sename
não fizer parte_Materials
se torna parte da interface da função. Isso significa que ele precisa fazer parte da documentação da interface e, se você decidir alterar essa exceção no futuro, será uma alteração na interface .A questão mais interessante é o que acontece se for uma pré-condição. Nesse caso, a pré-condição em si se torna parte da interface da função, mas a maneira como uma função se comporta quando essa condição é violada não é necessariamente parte da interface.
Nesse caso, a abordagem que você postou, verificando violações de pré-condições e relatando o erro, é conhecida como programação defensiva . A programação defensiva é boa, pois notifica o usuário logo que cometeu um erro e chamou a função com um argumento falso. É ruim, pois aumenta significativamente a carga de manutenção, pois o código do usuário pode depender que a função lide com violações de pré-condição de uma certa maneira. Em particular, se você achar que a verificação de tempo de execução se tornará um gargalo de desempenho no futuro (improvável para um caso tão simples, mas bastante comum para pré-condições mais complexas), talvez não seja mais possível removê-la.
Acontece que essas desvantagens podem ser muito significativas, o que deu à programação defensiva uma má reputação em certos círculos. No entanto, o objetivo inicial ainda é válido: queremos que os usuários de nossa função notem mais cedo quando cometerem um erro.
Portanto, hoje em dia muitos desenvolvedores propõem uma abordagem ligeiramente diferente para esses tipos de problemas. Em vez de lançar uma exceção, eles usam um mecanismo semelhante a uma declaração para verificar pré-condições. Ou seja, as pré-condições podem ser verificadas nas compilações de depuração para ajudar os usuários a detectar erros desde o início, mas eles não fazem parte da interface de funções. A diferença pode parecer sutil à primeira vista, mas pode fazer uma enorme diferença na prática.
Tecnicamente, chamar uma função com pré-condições violadas é um comportamento indefinido. Mas a implementação pode decidir detectar esses casos e notificar o usuário imediatamente, se isso acontecer. Infelizmente, as exceções não são uma boa ferramenta para implementar isso, pois o código do usuário pode reagir a elas e, portanto, começar a depender da presença delas.
Para obter uma explicação detalhada dos problemas com a abordagem defensiva clássica, bem como uma possível implementação para verificações de pré-condição no estilo de afirmação, consulte a Programação Defensiva de John Lakos, realizada diretamente no CppCon 2014 ( Slides , Video ).
fonte
Já existem algumas respostas aqui, mas aqui está a resposta que leva em consideração o Unity3D (a resposta é muito específica para o Unity3D, eu faria isso muito diferentemente na maioria dos contextos):
Em geral, o Unity3D não usa exceções tradicionalmente. Se você lançar uma exceção no Unity3D, não será como em um aplicativo .NET comum, a saber, não interromperá o programa, na maioria das vezes você poderá configurar o editor para pausar. Apenas será registrado. Isso pode facilmente colocar o jogo em um estado inválido e criar um efeito em cascata que dificulta o rastreamento dos erros. Então, eu diria que, no caso da Unity, deixar
Add
lançar uma exceção é uma opção especialmente indesejável.Mas examinar a velocidade das exceções não é um caso de otimização prematura em alguns casos, devido ao modo como o Mono funciona no Unity em algumas plataformas. De fato, o Unity3D no iOS suporta algumas otimizações avançadas de script, e as exceções desabilitadas * são um efeito colateral de uma delas. Isso é realmente algo a considerar, porque essas otimizações se mostraram muito valiosas para muitos usuários, mostrando um argumento realista para considerar a limitação do uso de exceções no Unity3D. (* exceções gerenciadas do mecanismo, não seu código)
Eu diria que no Unity você pode querer adotar uma abordagem mais especializada. Ironicamente, um muito baixo votado resposta no momento da escrita deste , mostra uma maneira que eu poderia implementar algo parecido com isto especificamente no contexto da Unity3D (em outro lugar algo como isso realmente é inaceitável, e até mesmo em Unity é bastante deselegante).
Outra abordagem que eu consideraria na verdade não é indicar um erro no que diz respeito ao chamador, mas usar as
Debug.LogXX
funções. Dessa forma, você obtém o mesmo comportamento de lançar uma exceção não tratada (por causa de como o Unity3D lida com eles) sem correr o risco de colocar algo em um estado estranho na linha. Considere também se isso realmente é um erro (a tentativa de carregar o mesmo material duas vezes necessariamente é um erro no seu caso? Ou pode ser um caso ondeDebug.LogWarning
é mais aplicável).E no que diz respeito ao uso de coisas como
Debug.LogXX
funções em vez de exceções, você ainda precisa considerar o que acontece quando uma exceção é lançada de algo que retorna um valor (como GetMaterial). Eu costumo abordar isso passando nulo junto com o log do erro ( novamente, apenas no Unity ). Em seguida, uso verificações nulas em meus MonoBehaviors para garantir que qualquer dependência, como um material, não seja um valor nulo e desabilito o MonoBehavior, se for o caso. Um exemplo para um comportamento simples que requer algumas dependências é algo como isto:SceneData.GetValue<>
é semelhante ao seu exemplo, pois chama uma função em um dicionário que gera uma exceção. Mas, em vez de lançar uma exceção, ele usa,Debug.LogError
que fornece um rastreamento de pilha como uma exceção normal faria e retornaria nulo. As verificações a seguir * desabilitarão o comportamento em vez de permitir que ele continue a existir em um estado inválido.* os cheques se parecem com isso por causa de um pequeno auxiliar que eu uso que imprime uma mensagem formatada quando desativa o objeto do jogo **. Verificações nulas simples com
if
trabalho aqui (** as verificações do auxiliar são compiladas apenas nas compilações de depuração (como afirmações). O uso de lambdas e expressões como essa no Unity pode prejudicar o desempenho)fonte
Gosto das duas respostas principais, mas quero sugerir que os nomes das suas funções possam ser melhorados. Estou acostumado a Java, então YMMV, mas um método "add" não deve, IMO, lançar uma exceção se o item já estiver lá. Ele deve adicionar o item novamente ou, se o destino for um Conjunto, não faça nada. Como esse não é o comportamento de Materials.Add, ele deve ser renomeado como TryPut ou AddOnce ou AddOrThrow ou semelhante.
Da mesma forma, seu LoadMaterial deve ser renomeado para LoadIfAbsent ou Put ou TryLoad ou LoadOrThrow (dependendo de você usar a resposta nº 1 ou nº 2) ou algo parecido.
Siga as convenções de nomenclatura do C # Unity, sejam elas quais forem.
Isso será especialmente útil se você tiver outras funções AddFoo e LoadBar, nas quais é permitido carregar a mesma coisa duas vezes. Sem nomes claros, os desenvolvedores ficarão frustrados.
fonte
Todas as respostas acrescentam idéias valiosas, gostaria de combiná-las:
Decida a semântica pretendida e esperada da
LoadMaterial()
operação. Existem pelo menos as seguintes opções:Uma pré-condição em
name
∉ LoadedMaterials : →Quando a pré-condição é violada, o efeito de
LoadMaterial()
não é especificado (como em resposta pelo ComicSansMS ). Isso permite a maior liberdade na implementação e futuras alterações deLoadMaterial()
. Ou,efeitos da chamada
LoadMaterial(name)
comname
∈ LoadedMaterials são especificados; ou:a especificação afirma que o resultado é idempotente (como em resposta por Karl Bielefeldt ),
Ao decidir a semântica, você deve escolher uma implementação. As seguintes opções e considerações foram propostas:
Lance uma exceção personalizada (como sugerido pelo Ixrec ) →
Para evitar o custo de verificar repetidamente
name
∉ LoadedMaterials, você pode seguir o conselho de Marc van Leeuwen :Vamos Dictionay.Add lançar a exceção →
Embora a maioria dos eleitores concorde mais com o Ixrec
Razões adicionais para escolher esta implementação são:
ArgumentException
exceções eMas se esses dois motivos forem importantes, você também poderá derivar sua exceção personalizada
ArgumentException
e usar o original como uma exceção encadeada.Torne-o
LoadMaterial()
idempotente como resposta por Karl Bielefeldt e com mais votos (75 vezes).As opções de implementação para esse comportamento:
Verifique com Dictionary.ContainsKey ()
Sempre chame Dictionary.Add () captura o que
ArgumentException
ele lança quando a chave a inserir já existe e ignora essa exceção. Documente que ignorar a exceção é o que você pretende fazer e por quê.LoadMaterials()
é (quase) sempre chamado uma vez para cada umname
, isso evita o custo de verificar repetidamentename
∉ LoadedMaterials cf. Marc van Leeuwen . Contudo,LoadedMaterials()
muitas vezes é chamado várias vezes para o mesmoname
, isso incorre no custo (caro) de lançarArgumentException
e desenrolar a pilha.Pensei que existisse um
TryAdd
análogo -method para TryGet () que permitiria evitar a exceção cara de lançar e empilhar o desenrolamento de chamadas com falha para Dictionary.Add.Mas esse
TryAdd
método parece não existir.fonte
O código de lançamento da exceção é redundante.
Se você ligar para:
com a mesma tecla, ele lançará um
A mensagem será: "Um item com a mesma chave já foi adicionado."
A
ContainsKey
verificação é redundante, pois essencialmente o mesmo comportamento é alcançado sem ela. O único item diferente é a mensagem real na exceção. Se houver um verdadeiro benefício em receber uma mensagem de erro personalizada, o código de proteção terá algum mérito.Caso contrário, eu provavelmente refatoraria a guarda neste caso.
fonte
Eu acho que isso é mais uma questão sobre o design da API do que uma questão de convenção de codificação.
Qual é o resultado esperado (o contrato) da chamada:
Se o chamador pode esperar / é garantido que, depois de chamar esse método, o material "madeira" seja carregado, não vejo motivo para lançar uma exceção quando esse material já estiver carregado.
Se ocorrer um erro ao carregar o material, por exemplo, a conexão com o banco de dados não pôde ser aberta ou não houver material "madeira" no repositório, é correto lançar uma exceção para notificar o responsável pela chamada sobre esse problema.
fonte
Por que não mudar o método para que ele retorne 'true' se a matéria for adicionada e 'false' se não foi?
fonte