Use um else depois da exceção (ou não)

9

Considere este pedaço de código:

if (x == 1)
{
  throw "no good; aborting" ;
}

[... more code ...]

Agora considere este código:

if (x == 1)
{
  throw "no good; aborting" ;
}
else
{
  [... more code ...]
}

Os dois casos funcionam exatamente da mesma maneira. O primeiro caso tem a vantagem de que você não precisa "envolver" o restante do código em um arquivo else. O segundo tem a vantagem de seguir a prática de ter explicitamente um elsepara todos if.

Alguém pode apresentar argumentos sólidos em favor de um sobre o outro?

rlandster
fonte
5
A prática que você cita a favor do explícito elseparece falsa. Muitas vezes, simplesmente não há nada para colocar no elsebloco, a menos que você se incline para trás.
Consistência? Permitir robustez diante da mudança de código? Legibilidade?
Thomas Eding
11
Ehhhh, eu não sou muito fã da idéia de que todo mundo ifprecisa de um else. O último programador que trabalhou em nossa base de código seguiu isso rigidamente ( bem, às vezes ... é meio esquizofrênico ). Como resultado, temos um monte de totalmente sem sentido else { /* do nothing */ }blocos desarrumar o código ...
KChaloux
4
"An else for every if" parece uma proclamação bizarra emitida por um arquiteto de nuvem em nome da consistência (tola). Não vejo vantagem em seguir essa prática e nunca ouvi falar dela em lugar algum.
Erik Dietrich
É outra coisa redundante. Se você estiver trabalhando com a pilha .NET, instale o ReSharper e ele lembrará você de remover todas as instruções else redundantes.
CodeART 30/10/12

Respostas:

16

Você não deve adicionar ramificações elseposteriores ifque quebram o fluxo de controle incondicionalmente , como aquelas que contêm a throwou a return. Isso melhoraria a legibilidade do seu programa, removendo o nível desnecessário de aninhamento introduzido pela elseramificação.

O que parece mais ou menos OK com um single throwtorna-se realmente feio quando vários lançamentos consecutivos são realizados:

void myMethod(int arg1, int arg2, int arg3) {
    // This is demonstrably ugly - do not code like that!
    if (!isValid(arg1)) {
        throw new ArgumentException("arg1 is invalid");
    } else {
        if (!isValid(arg2)) {
            throw new ArgumentException("arg2 is invalid");
        } else {
            if (!isValid(arg3)) {
                throw new ArgumentException("arg3 is invalid");
            } else {
                // The useful code starts here
            }
        }
    }
}

Este trecho faz a mesma coisa, mas parece muito melhor:

void myMethod(int arg1, int arg2, int arg3) {
    if (!isValid(arg1)) {
        throw new ArgumentException("arg1 is invalid");
    }
    if (!isValid(arg2)) {
        throw new ArgumentException("arg2 is invalid");
    }
    if (!isValid(arg3)) {
        throw new ArgumentException("arg3 is invalid");
    }
    // The useful code starts here
}
dasblinkenlight
fonte
+1 Verdadeiro. O segundo caso do OP obriga você a ler atentamente e, em seguida, deixa um WTF. Mas ... sempre tente encurtar os métodos. Um retorno no meio de um método de 200 linhas também é ruim.
Tulains Córdova 30/10/12
11
Para ser justo, se você está apenas usando repetidos if, pode fazer else if.
Guvante 30/10/12
2
@ Guvante: Cada ifteste para uma única condição e lida com ela se a condição for verdadeira, e a menos que exista algo alternativo que ocorra se a condição for falsa, else ifsão desnecessários. Temos um prazo em meu escritório para código como o primeiro trecho de dasblinkenlight: " máquinas pachinko ".
Blrfl
@Blrfl pachinko machines hah, analogia perfeita +1 #
Jimmy Hoffa
@ Blrfl: Eu estava referenciando que ifs repetidos são um mau exemplo de aninhamento demais. Você não deve aninhar ifs repetidos de qualquer maneira. Concordo que, em geral, a menos que você esteja falando de uma quantidade menor de código, não há motivo para incluí-lo else.
Guvante 30/10/12
5

Eu chamaria a prática de "outra coisa explícita" a que você se refere como anti-padrão, pois obscurece o fato de que não há código de caso especial como outra coisa para o seu if.

A legibilidade / manutenção geralmente é aprimorada quando você não possui nada além de construções de fluxo de código necessárias e você as minimiza. Isso significa erros redundantes e ifs que adicionarão um escopo a uma função inteira dificultam o acompanhamento e a manutenção.

Digamos, por exemplo, que você tenha esta função:

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (_validColors.Contains(oblogonToConfigure.Color))
    {
        oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
    }
    else
    {
        oblogonToConfigure.Color = _validColors[0];
        oblogonToConfigure.ColorIndex = 0;
    }
}

Agora, existe o requisito de que, durante a configuração, você também deve especificar o índice de tipo / tipo do oblogon, existem vários escopos nos quais alguém pode colocar esse código e obter um código inválido, ou seja,

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (!_validOblogons.Contains(oblogonToConfigure.Type))
    {
        oblogonToConfigure.Type = _validOblogons[0];
        oblogonToConfigure.TypeIndex = 0;
        if (_validColors.Contains(oblogonToConfigure.Color))
        {
            oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
        }
        else
        {
            oblogonToConfigure.Color = _validColors[0];
            oblogonToConfigure.ColorIndex = 0;
        }
    }
    else
    {
        oblogonToConfigure.TypeIndex = _validOblogons.IndexOf(oblogonToConfigure.Type);
    }
}

Compare isso se o código original foi escrito com construções mínimas de fluxo de controle necessárias e minimizadas.

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (!_validColors.Contains(oblogonToConfigure.Color))
    {
        oblogonToConfigure.Color = _validColors[0];
    }

    oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
}

Agora seria muito mais difícil colocar acidentalmente algo no escopo errado ou acabar inchando os escopos, causando duplicação no crescimento e manutenção a longo prazo dessa função. Além disso, é óbvio quais são os possíveis fluxos através dessa função para que a legibilidade seja aprimorada.

Eu sei, o exemplo é um pouco artificial, mas eu já vi muitas vezes

SomeFunction()
{
    if (isvalid)
    {
        /* ENTIRE FUNCTION */
    }
    /* Nothing should go here but something does on accident, and an invalid scenario is created. */
}

Então, formalizando essas regras sobre construções de controle-fluxo, acho que pode ajudar as pessoas a desenvolver a intuição necessária para sentir o cheiro de algo quando começarem a escrever código assim. Então eles vão começar a escrever ..

SomeFunction()
{
    if (!isvalid)
    {
        /* Nothing should go here, and it's so small no one will likely accidentally put something here */
        return;
    }

    /* ENTIRE FUNCTION */
}
Jimmy Hoffa
fonte