ti-enxame.com

Invertendo uma instrução IF

Então, eu estou programando há alguns anos e recentemente comecei a usar o ReSharper mais. Uma coisa que o ReSharper sempre me sugere é "inverter 'se' para reduzir o aninhamento".

Digamos que eu tenho esse código:

foreach (someObject in someObjectList) 
{
    if(someObject != null) 
    {
        someOtherObject = someObject.SomeProperty;
    }
}

E o ReSharper sugerirá que eu faça isso:

foreach (someObject in someObjectList)
{
    if(someObject == null) continue;
    someOtherObject = someObject.SomeProperty;
}

Parece que o ReSharper SEMPRE sugere que eu inverta os IFs, não importa quanto aninhamento esteja acontecendo. Esse problema é que eu meio que gosto de aninhar em pelo menos algumas situações. Para mim, parece mais fácil ler e descobrir o que está acontecendo em certos lugares. Nem sempre é esse o caso, mas às vezes me sinto mais confortável ao aninhar.

Minha pergunta é: além da preferência pessoal ou legibilidade, existe um motivo para reduzir o aninhamento? Existe uma diferença de desempenho ou outra coisa que eu possa não estar ciente?

39
Barry Franklin

Depende. No seu exemplo, ter a instrução if não invertida não é um problema, porque o corpo da if é muito curto. No entanto, você geralmente deseja inverter as declarações quando os corpos crescerem.

Somos apenas seres humanos e manter várias coisas ao mesmo tempo em nossas cabeças é difícil.

Quando o corpo de uma declaração é grande, invertê-la não apenas reduz o aninhamento, mas também informa ao desenvolvedor que eles podem esquecer tudo o que está acima da declaração e se concentrar apenas no restante. É muito provável que você use as instruções invertidas para se livrar de valores errados o mais rápido possível. Depois que você souber que eles estão fora do jogo, a única coisa que resta são valores que realmente podem ser processados.

64
Andy

Não evite negativos. Os seres humanos sugam como analisá-los, mesmo quando a CPU os ama.

No entanto, respeite os idiomas. Nós, seres humanos, somos criaturas de hábitos, por isso preferimos caminhos bem gastos a caminhos diretos cheios de ervas daninhas.

foo != null, Infelizmente, tornou-se um idioma. Eu mal percebo mais as partes separadas. Eu olho para isso e penso: "oh, é seguro destacar foo agora".

Se você quiser derrubar isso (oh, um dia, por favor), precisará de um argumento realmente forte. Você precisa de algo que permita que meus olhos deslizem sem esforço e entendam em um instante.

No entanto, o que você nos deu foi o seguinte:

foreach (someObject in someObjectList)
{
    if(someObject == null) continue;
    someOtherObject = someObject.SomeProperty;
}

O que nem é estruturalmente o mesmo!

foreach (someObject in someObjectList)
{
    if(someObject == null) continue;
    someOtherObject = someObject.SomeProperty;

    if(someGlobalObject == null) continue;
    someSideEffectObject = someGlobalObject.SomeProperty;
}

Isso não está fazendo o que meu cérebro preguiçoso esperava. Eu pensei que poderia continuar adicionando código independente, mas não posso. Isso me faz pensar em coisas que não quero pensar agora. Você quebrou meu idioma, mas não me deu um melhor. Tudo porque você queria me proteger daquele negativo que queimou, é um desagradável ego em minha alma.

Desculpe, talvez o ReSharper esteja certo ao sugerir isso 90% das vezes, mas em verificações nulas, acho que você encontrou um caso de esquina onde é melhor ignorá-lo. As ferramentas simplesmente não são boas para ensinar o que é um código legível. Pergunte a um humano. Faça uma revisão por pares. Mas não siga cegamente a ferramenta.

33
candied_orange

É o velho argumento sobre se uma pausa é pior do que o aninhamento.

As pessoas parecem aceitar retorno antecipado para verificações de parâmetros, mas isso é desaprovado na maior parte do método.

Pessoalmente, evito continuar, quebrar, ir etc., mesmo que isso signifique mais aninhamento. Mas na maioria dos casos, você pode evitar os dois.

foreach (someObject in someObjectList.where(i=>i != null))
{
    someOtherObject = someObject.SomeProperty;
}

Obviamente, o aninhamento dificulta as coisas quando você tem muitas camadas

foreach (someObject in someObjectList) 
{
    if(someObject != null) 
    {
        someOtherObject = someObject.SomeProperty;
        if(someObject.x > 5) 
        {
            x ++;
            if(someObject.y > 5) 
            {
                x ++;
            }
        }
    }
}

O pior é quando você tem ambos

foreach (someObject in someObjectList) 
{
    if(someObject != null) 
    {
        someOtherObject = someObject.SomeProperty;
        if(someObject.x > 5) 
        {
            x ++;
            if(someObject.y > 5) 
            {
                x ++;
                return;
            }
            continue;
        }
        someObject.z --;
        if(myFunctionWithSideEffects(someObject) > 6) break;
    }
}
20
Ewan

O problema com o continue é que, se você adicionar mais linhas ao código, a lógica será complicada e provavelmente conterá erros. por exemplo.

foreach (someObject in someObjectList)
{
    if(someObject == null) continue;
    someOtherObject = someObject.SomeProperty;

    ...  imagine that there is some code here ...

    // added later by a Junior Developer
    doSomeOtherFunction(someObject);
}

Deseja chamar doSomeOtherFunction() para all someObject, ou apenas se não for nulo? Com o código original, você tem a opção e é mais clara. Com o continue pode-se esquecer "oh, sim, lá atrás, pulamos nulos" e você nem tem a opção de chamá-lo para todos os someObjects, a menos que você faça essa chamada no início do método que pode não ser possível ou correto por outros motivos.

Sim, muitos aninhamentos são feios e possivelmente confusos, mas nesse caso eu usaria o estilo tradicional.

Pergunta bônus: Por que existem nulos nessa lista para começar? Talvez seja melhor nunca adicionar um nulo em primeiro lugar; nesse caso, a lógica de processamento é muito mais simples.

6
user949300

A ideia é o retorno antecipado. O início return em um loop se torna adiantado continue.

Muitas respostas boas para esta pergunta: Devo retornar de uma função mais cedo ou usar uma instrução if?

Observe que, embora a questão seja encerrada com base em opiniões, mais de 430 votos são a favor do retorno antecipado, ~ 50 votos são "depende" e 8 são contra o retorno antecipado. Portanto, existe um consenso excepcionalmente forte (ou isso, ou sou mau em contar, o que é plausível).

Pessoalmente, se o corpo do loop estiver sujeito a continuação antecipada e tempo suficiente para importar (3-4 linhas), eu tendem a extrair o corpo do loop em uma função em que eu uso o retorno antecipado em vez de continuar antecipadamente.

3
Peter

Essa técnica é útil para certificar pré-condições quando a maior parte do seu método ocorre quando essas condições são atendidas.

Freqüentemente invertemos ifs no meu trabalho e empregamos outro fantasma. Costumava ser bastante frequente que, ao revisar um PR, eu pudesse apontar como meu colega poderia remover três níveis de aninhamento! Isso acontece com menos frequência, já que lançamos nossos ifs.

Aqui está um exemplo de brinquedo que pode ser implementado

function foobar(x, y, z) {
    if (x > 0) {
        if (z != null && isGamma(y)) {
            // ~10 lines of code
            // return something
        }
        else {
           return y
        }
    }
    else {
      return y + z
    }
}

Código como este, onde há UM caso interessante (onde o resto são simplesmente retornos ou pequenos blocos de instrução) são comuns. É trivial reescrever o item acima como

function foobar(x, y, z) {
    if (x <=0) {
        return y + z
    }
    if (z == null || !isGamma(y) {
       return y
    }
    // ~ 10 lines of code
    // return something
}

Aqui, nosso código significativo, as 10 linhas não incluídas, não é recuado três vezes na função. Se você tem o primeiro estilo, é tentado a refatorar o bloco longo em um método ou a tolerar o recuo. É nesse ponto que a economia entra em cena. Em um local, essa é uma economia trivial, mas em muitos métodos em várias classes, você economiza a necessidade de gerar uma função extra para tornar o código mais limpo e você não tem muito recuo hediondo em vários níveis.


Em resposta ao exemplo de código do OP, concordo que esse é um colapso excessivo. Especialmente porque em 1 TB, no estilo que uso, a inversão faz com que o código aumente de tamanho.

2
Lan

Eu acho que o ponto mais importante aqui é que o objetivo do loop determina a melhor maneira de organizar o fluxo dentro do loop. Se você estiver filtrando alguns elementos antes de percorrer o restante, continue sair cedo faz sentido. No entanto, se você estiver executando diferentes tipos de processamento em um loop, pode fazer mais sentido tornar isso if realmente óbvio, como:

for(obj in objectList) {
    if(obj == null) continue;
    if(obj.getColour().equals(Color.BLUE)) {
        // Handle blue objects
    } else {
        // Handle non-blue objects
    }
}

Observe que em Java Streams ou outros idiomas funcionais, você pode separar a filtragem do loop, usando:

items.stream()
    .filter(i -> (i != null))
    .filter(i -> i.getColour().equals(Color.BLUE))
    .forEach(i -> {
        // Process blue objects
    });

Aliás, essa é uma das coisas que eu amo sobre o Perl invertido se ( unless ) aplicado a instruções únicas, o que permite o seguinte tipo de código, que eu acho muito fácil de ler:

foreach my $item (@items) {
    next unless defined $item;
    carp "Only blue items are supported! Failed on item $item" 
       unless ($item->get_colour() eq 'blue');
    ...
}
0
Gaurav

As sugestões de novos compartilhadores de opinião geralmente são úteis, mas devem sempre ser avaliadas criticamente. Ele procura alguns padrões de código predefinidos e sugere transformações, mas não pode levar em consideração todos os fatores que tornam o código mais ou menos legível para humanos.

Sendo todas as outras coisas iguais, menos aninhamento é preferível, e é por isso que o ReSharper sugere uma transformação que reduz o aninhamento. Mas todas as outras coisas são não iguais: Nesse caso, a transformação requer a introdução de um continue, o que significa que o código resultante é realmente mais difícil de seguir.

Em alguns casos, a troca de um nível de aninhamento por um continuepoderia realmente é uma melhoria, mas isso depende da semântica do código em questão, que está além do entendimento das regras mecânicas do ReSharpers.

No seu caso, a sugestão do ReSharper tornaria o código pior. Então apenas ignore. Ou melhor: não use a transformação sugerida, mas pense um pouco sobre como o código pode ser aprimorado. Observe que você pode estar atribuindo a mesma variável várias vezes, mas apenas a última atribuição terá efeito, pois a anterior seria substituída. Este faz parece um bug. A sugestão ReSharpers não corrige o bug, mas torna um pouco mais óbvio que alguma lógica dúbia está acontecendo.

0
JacquesB