Esta é uma pequena coisinha, mas toda vez que tenho que codificar algo assim, a repetição me incomoda, mas não tenho certeza de que nenhuma das soluções seja pior.
if(FileExists(file))
{
contents = OpenFile(file); // <-- prevents inclusion in if
if(SomeTest(contents))
{
DoSomething(contents);
}
else
{
DefaultAction();
}
}
else
{
DefaultAction();
}
- Existe um nome para esse tipo de lógica?
- Eu sou um pouco TOC demais?
Estou aberto a sugestões de códigos malignos, mesmo que por curiosidade ...
coding-style
conditions
Benjol
fonte
fonte
DefaultAction
chamadas viola o princípio DRYmake sure I have valid data for DoSomething(), and then DoSomething() with it. Otherwise, take DefaultAction()
. Os detalhes detalhados de se ter os dados de DoSomething () estão em um nível de abstração mais baixo e, portanto, devem estar em uma função diferente. Esta função terá um nome no nível de abstração mais alto e sua implementação será de baixo nível. As boas respostas abaixo abordam esse problema.Respostas:
Extraia-o para separar a função (método) e use a
return
instrução:Ou, talvez melhor, separe a obtenção de conteúdo e seu processamento:
Upd:
Por que não exceções, por
OpenFile
que não lança exceção de E / S:Eu acho que é uma pergunta realmente genérica, em vez de uma pergunta sobre E / S de arquivo. Nomes como
FileExists
,OpenFile
podem ser confusos, mas se substituí-los porFoo
,Bar
etc, - seria mais claro queDefaultAction
pode ser chamado tantas vezes quantoDoSomething
, portanto, pode ser um caso não excepcional. Péter Török escreveu sobre isso no final de sua respostaPor que existe um operador condicional ternário na 2ª variante:
se houvesse tag [C ++], eu escrevi uma
if
declaração com declaraçãocontents
na parte de sua condição:Porém, para outras linguagens (tipo C),
if(contents) ...; else ...;
é exatamente o mesmo que a expressão expression com operador condicional ternário, mas mais longo. Como a parte principal do código eraget_contents
função, usei apenas a versão mais curta (e também ocontents
tipo omitido ). Enfim, está além dessa questão.fonte
Se a linguagem de programação que você está usando (0) comparações binárias de curto-circuito (ou seja, se não chamar
SomeTest
seFileExists
retornar falso) e (1) a atribuição retornar um valor (o resultado deOpenFile
é atribuído acontents
e então esse valor é passado como argumento paraSomeTest
), você pode usar algo como o seguinte, mas ainda assim seria recomendável comentar o código, observando que o single=
é intencional.Dependendo da complexidade do if, pode ser melhor ter uma variável de flag (que separa o teste de condições de sucesso / falha com o código que lida com o erro
DefaultAction
nesse caso)fonte
if
declaração, na minha opinião.Mais seriamente do que a repetição da chamada para DefaultAction é o próprio estilo, porque o código é escrito não ortogonal (veja esta resposta por boas razões para escrever ortogonalmente).
Para mostrar por que o código não ortogonal é ruim, considere o exemplo original, quando é introduzido um novo requisito de que não devemos abrir o arquivo se ele estiver armazenado em um disco de rede. Bem, podemos apenas atualizar o código para o seguinte:
Mas também existe um requisito de que também não devemos abrir arquivos grandes com mais de 2 GB. Bem, apenas atualizamos novamente:
Deve ficar muito claro que esse estilo de código será uma enorme dor de manutenção.
Entre as respostas aqui escritas corretamente ortogonalmente estão o segundo exemplo de Abyx e a resposta de Jan Hudec , por isso não vou repetir isso, basta apontar que a adição dos dois requisitos nessas respostas seria apenas
(ou em
goto notexists;
vez dereturn null;
), não afetando nenhum outro código além das linhas adicionadas . Por exemplo, ortogonal.Ao testar, a regra geral deve ser testar exceções, não o caso normal .
fonte
Obviamente:
Você disse que está aberto até a soluções más, então, usando o mal goto conta, não?
De fato, dependendo do contexto, essa solução pode ser menos mal do que o mal fazendo a ação duas vezes ou a variável extra do mal. Eu o envolvi em uma função, porque definitivamente não seria bom no meio da função longa (principalmente devido ao retorno no meio). Mas que a função longa não está OK, ponto final.
Quando você tiver exceções, elas serão mais fáceis de ler, especialmente se você puder fazer com que o OpenFile e DoSomething simplesmente joguem uma exceção se as condições não forem atendidas, para que você não precise de verificações explícitas. Por outro lado, em C ++, Java e C # lançando uma exceção é uma operação lenta, portanto, do ponto de desempenho, o goto ainda é preferível.
Nota sobre "evil": C ++ FAQ 6.15 define "evil" como:
E isso se aplica
goto
neste contexto. Construções estruturadas de controle de fluxo são melhores na maioria das vezes, mas quando você encontra uma situação em que eles acumulam muitos de seus próprios males, como atribuição em condição, aninhando mais de 3 níveis de profundidade, código duplicado ou condições longas,goto
pode simplesmente terminar sendo menos mau.fonte
for(;;) { if(!FileExists(file)) break; contents = OpenFile(file); if(!SomeTest(contents)) break; DoSomething(contents); return; } /* broken out */ DefaultAction();
goto
, porque você está abusandobreak
de uma maneira que ninguém espera que seja abusada; portanto, as pessoas que leem o código terão mais problemas para ver onde a ruptura os leva do que com goto, que diz explicitamente. Além disso, você está usando um loop infinito que será executado apenas uma vez, o que será bastante confuso. Infelizmente,do { ... } while(0)
também não é exatamente legível, porque você só vê que é apenas um bloco engraçado quando você chega ao final e C não suporta quebra de outros blocos (ao contrário do perl)....
fonte
SomeTest
pode ter a mesma semântica que a existência de arquivo seSomeTest
verificar o tipo de arquivo, por exemplo, verificar se .gif é realmente um arquivo GIF.contents && f(contents)
. Duas funções para salvar mais uma ?!Uma possibilidade:
Obviamente, isso torna o código um pouco mais complexo de uma maneira diferente. Portanto, é em grande parte uma questão de estilo.
Uma abordagem diferente seria usar exceções, por exemplo:
Isso parece mais simples, porém só é aplicável se
DefaultAction()
se encaixa em cadaSomeTest()
seja claramente uma condição incorreta; portanto, é adequado lançar uma exceção nele.fonte
(function () { ... })()
em Javascript,{ flag = false; ... }
em C-like etc.Isso está em um nível mais alto de abstração:
E isso preenche os detalhes.
fonte
Algumas pessoas acham essa abordagem um pouco extremada, mas também é muito limpa. Permita-me ilustrar em Python:
Quando ele diz que funções devem fazer uma coisa, ele quer dizer uma coisa.
processFile()
escolhe uma ação com base no resultado de um teste, e é tudo o que faz.fileMeetsTest()
combina todas as condições do teste, e é tudo o que faz.contentsTest()
transfere a primeira linha parafirstLineTest()
e é tudo o que faz.Parece um monte de funções, mas é praticamente parecido com o inglês direto:
É um pouco prolixo, mas observe que se um mantenedor não se importa com os detalhes, ele pode parar de ler após apenas as 4 linhas de código inseridas
processFile()
e ainda terá um bom conhecimento de alto nível sobre o que a função faz.fonte
Com relação ao que é chamado, ele pode se transformar facilmente no padrão anti-ponta de seta à medida que seu código cresce para lidar com mais requisitos (como mostra a resposta fornecida em https://softwareengineering.stackexchange.com/a/122625/33922 ) e então cai na armadilha de ter grandes seções de códigos com instruções condicionais aninhadas que se assemelham a uma seta.
Veja Links como;
http://codinghorror.com/blog/2006/01/flattening-arrow-code.html
http://lostechies.com/chrismissal/2009/05/27/anti-patterns-and-worst-practices-the-arrowhead-anti-pattern/
Há muito mais sobre esse e outros anti-padrões encontrados no Google.
Algumas ótimas dicas que Jeff fornece em seu blog sobre isso são;
Veja também alguns dos comentários no blog de Jeff sobre as sugestões de Steve McConnells sobre retorno antecipado;
Eu sempre assinei a teoria de 1 entrada / saída por função, devido ao que me ensinaram há 15 anos. Eu sinto que isso torna o código muito mais fácil de ler e, como você menciona, é mais sustentável
fonte
Isso está de acordo com as regras DRY, no-goto e no-multiple-return, é escalável e legível na minha opinião:
fonte
if
s dentro de outrosif
s. Além disso, o código para lidar com o caso malsucedido (DefaultAction()
) está apenas em um local e, para fins de depuração, o código não pula as funções auxiliares e adiciona pontos de interrupção às linhas nas quais asuccess
variável é alterada pode mostrar rapidamente quais testes foram aprovados (acima do acionado ponto de interrupção) e quais não foram testados (abaixo).success
paraok_so_far
:)Eu o extrairia para um método separado e, em seguida:
o que também permite
então, você pode remover as
DefaultAction
chamadas e deixar a execuçãoDefaultAction
do chamador:Também gosto da abordagem de Jeanne Pindar .
fonte
Para este caso em particular, a resposta é fácil o suficiente ...
Há uma condição de corrida entre
FileExists
eOpenFile
: o que acontece se o arquivo for removido?A única maneira sensata de lidar com esse caso específico é pular
FileExists
:Isso resolve perfeitamente esse problema e torna o código mais limpo.
Em geral: tente repensar o problema e invente outra solução que evite o problema completamente.
fonte
Outra possibilidade, se você não gosta de ver muitos outros, é abandonar completamente o uso do else e lançar uma declaração de retorno extra. Else é uma espécie de supérfluo, a menos que exigem lógica mais complexa para determinar se há mais do que apenas duas possibilidades de ação.
Assim, seu exemplo pode se tornar:
Pessoalmente, não me importo de usar a cláusula else , pois ela afirma explicitamente como a lógica deve funcionar e, portanto, melhora a legibilidade do seu código. Algumas ferramentas de embelezamento de código, no entanto, preferem simplificar para uma única instrução if para desencorajar a lógica de aninhamento.
fonte
O caso mostrado no código de exemplo geralmente pode ser reduzido para uma única
if
instrução. Em muitos sistemas, a função de abertura de arquivo retornará um valor inválido se o arquivo ainda não existir. Às vezes, esse é o comportamento padrão; outras vezes, ele deve ser especificado por meio de um argumento. Isso significa que oFileExists
teste pode ser descartado, o que também pode ajudar nas condições de corrida resultantes da exclusão do arquivo entre o teste de existência e a abertura do arquivo.Isso não aborda diretamente o problema da mistura no nível de abstração, pois evita completamente o problema de vários testes não encadeados, embora acabar com o teste de existência de arquivo não seja incompatível com a separação dos níveis de abstração. Supondo que identificadores de arquivo inválidos sejam equivalentes a "false" e identificadores de arquivo fecham quando ficam fora do escopo:
fonte
Estou de acordo com frozenkoi, no entanto, para C # de qualquer maneira, pensei que ajudaria a seguir a sintaxe dos métodos TryParse.
fonte
Seu código é feio porque você está fazendo muito em uma única função. Você deseja processar o arquivo ou executar a ação padrão. Comece dizendo que:
Programadores Perl e Ruby escrevem
processFile(file) || defaultAction()
Agora vá escrever ProcessFile:
fonte
Claro que você só pode ir tão longe em cenários como estes, mas aqui está um caminho a percorrer:
Você pode querer filtros adicionais. Então faça o seguinte:
Embora isso possa fazer sentido também:
Qual é o melhor? Isso depende do problema do mundo real que você está enfrentando.
Mas o que se deve tirar é: você pode fazer muito com composição e polimorfismo.
fonte
O que há de errado com o óbvio
Parece-me bastante normal? Para esse tipo de procedimento grande, onde muitas pequenas coisas precisam acontecer, a falha de qualquer uma delas impediria a segunda. As exceções o tornam um pouco mais limpo, se for uma opção.
fonte
Percebo que essa é uma pergunta antiga, mas notei um padrão que não foi mencionado; principalmente, definir uma variável para determinar posteriormente o (s) método (s) que você gostaria de chamar (fora do if ... else ...).
Esse é outro ângulo para facilitar o trabalho do código. Também permite quando você pode adicionar outro método a ser chamado ou alterar o método apropriado que precisa ser chamado em determinadas situações.
Em vez de precisar substituir todas as menções do método (e talvez perder alguns cenários), todas elas são listadas no final do bloco if ... else ... e são mais simples de ler e alterar. Costumo usar isso quando, por exemplo, vários métodos podem ser chamados, mas dentro do aninhado se ... else ... um método pode ser chamado em várias correspondências.
Se você definir uma variável que define o estado, poderá ter muitas opções profundamente aninhadas e atualizar o estado quando algo deve ou não ser executado.
Isso pode ser usado como no exemplo da pergunta em que estamos verificando se 'DoSomething' ocorreu e, se não, execute a ação padrão. Ou você pode ter um estado para cada método que você deseja chamar, definir quando aplicável e chamar o método aplicável fora do if ... else ...
No final das instruções aninhadas if ... else ..., você verifica o estado e age de acordo. Isso significa que você precisa apenas de uma única menção de um método em vez de todos os locais em que ele deve ser aplicado.
fonte
Para reduzir o IF aninhado:
1 / retorno antecipado;
2 / expressão composta (atento a curto-circuito)
Portanto, seu exemplo pode ser refatorado assim:
fonte
Eu vi muitos exemplos com "return", que também uso, mas às vezes quero evitar a criação de novas funções e usar um loop:
Se você deseja escrever menos linhas ou odeia loops infinitos como eu, pode alterar o tipo de loop para "do ... while (0)" e evitar a última "interrupção".
fonte
Que tal esta solução:
Eu assumi que o OpenFile retorna um ponteiro, mas isso também pode funcionar com o retorno do tipo de valor, especificando algum valor padrão não retornável (códigos de erro ou algo parecido).
É claro que não espero alguma ação possível por meio do método SomeTest no ponteiro NULL (mas você nunca sabe), portanto isso também pode ser visto como uma verificação extra do ponteiro NULL para a chamada SomeTest (conteúdo).
fonte
Claramente, a solução mais elegante e concisa é usar uma macro de pré-processador.
O que permite escrever um código bonito como este:
Pode ser difícil confiar na formatação automática se você usar essa técnica com frequência, e alguns IDEs podem gritar um pouco com você sobre o que ela supõe erroneamente estar malformado. E como diz o ditado, tudo é uma troca, mas suponho que não seja um preço ruim a pagar para evitar os males do código repetido.
fonte
Como você perguntou por curiosidade e sua pergunta não está marcada com um idioma específico (mesmo que esteja claro que você tinha idiomas imperativos em mente), vale a pena acrescentar que os idiomas que suportam a avaliação lenta permitem uma abordagem completamente diferente. Nessas linguagens, as expressões são avaliadas apenas quando necessário, para que você possa definir "variáveis" e usá-las somente quando fizer sentido. Por exemplo, em uma linguagem de ficção com preguiçosos
let
/in
estruturas de você esquecer controle de fluxo e escreve:fonte