Escada If-else que deve pegar todas as condições - uma cláusula final redundante deve ser adicionada?

Isso é algo que estou fazendo muito ultimamente.

Exemplo:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else if (i > current) {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }
}

Muitas vezes, a escada if / else é significativamente mais complicada do que isso ...

Veja a cláusula final? É redundante. A escada deve pegar todas as condições possíveis. Assim, poderia ser reescrito assim:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }
}

Era assim que eu escrevia código, mas não gosto desse estilo. Minha reclamação é que a condição sob a qual a última parte do código será executada não é óbvia a partir do código. Assim, comecei a escrever essa condição explicitamente para torná-la mais evidente.

Contudo:

• Escrever explicitamente a condição exaustiva final é minha, e eu tenho más experiências com minhas próprias idéias - geralmente as pessoas gritam comigo sobre o quão horrível o que estou fazendo é - e (às vezes muito) depois, eu descobri que era realmente subótimo;
• Uma dica de por que isso pode ser uma péssima idéia: não aplicável ao Javascript, mas em outros idiomas, os compiladores tendem a emitir avisos ou mesmo erros sobre o controle que atinge o fim da função. Sugerir que algo assim pode não ser muito popular ou estou fazendo errado.
  • As reclamações do compilador me fizeram às vezes escrever a condição final em um comentário, mas acho que isso é horrível, pois os comentários, ao contrário do código, não têm efeito na semântica real do programa:

    } else { // i > current
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }

Estou esquecendo de algo? Ou é bom fazer o que descrevi ou é uma má ideia?

Ambas as abordagens são válidas. Mas vamos dar uma olhada em prós e contras.

Para uma ifcadeia com condições triviais como aqui, isso realmente não importa:

• com uma final else, é óbvio para o leitor descobrir em que condição o outro é acionado;
• com uma final else if, é tão óbvio para o leitor que não elseé necessário mais nada, pois você cobriu tudo.

No entanto, existem muitas ifcadeias que dependem de condições mais complexas, combinando estados de várias variáveis, talvez com uma expressão lógica complexa. Nesse caso, é menos óbvio. E aqui a consequência de cada estilo:

• final else: você tem certeza de que um dos galhos está ocupado. Se você esqueceu um caso, ele passará pelo último ramo; portanto, durante a depuração, se o último ramo foi escolhido e você esperava outra coisa, você descobrirá rapidamente.
• final else if: você precisa derivar a condição redundante para codificar, e isso cria uma possível fonte de erro com o reisk de não cobrir todos os casos. Além disso, se você perdeu um caso, nada será executado e poderá ser mais difícil descobrir que algo está faltando (por exemplo, se algumas variáveis ​​que você esperava definir mantiverem os valores das iterações anteriores).

Portanto, a condição redundante final é uma fonte de risco. É por isso que prefiro sugerir uma final else.

Editar: codificação de alta confiabilidade

Se você estiver desenvolvendo com alta confiabilidade em mente, poderá estar interessado em outra variante: concluir sua final explícita redundante else ifcom uma final elsepara capturar qualquer situação inesperada.

Essa é uma codificação defensiva. É recomendado por algumas especificações de segurança, como SEI CERT ou MISRA . Algumas ferramentas de análise estática ainda implementam isso como uma regra que é sistematicamente verificada (isso pode explicar os avisos do seu compilador).

Algo que está faltando nas respostas até agora é uma questão de que tipo de falha é menos prejudicial.

Se sua lógica é boa, não importa realmente o que você faz, o caso importante é o que acontece se você tiver um bug.

Você omite a condicional final: a opção final é executada mesmo que não seja a coisa certa a fazer.

Você simplesmente adiciona a condicional final: ela não executa nenhuma opção, dependendo da situação, isso pode significar simplesmente que algo falha na exibição (baixo dano) ou pode significar uma exceção de referência nula em algum momento posterior (que pode ser uma depuração dor.)

Você adiciona a condicional final e uma exceção: lança.

Você precisa decidir qual dessas opções é a melhor. No código de desenvolvimento, considero isso um acéfalo - considere o terceiro caso. No entanto, eu provavelmente definiria circle.src como uma imagem de erro e circle.alt como uma mensagem de erro antes de ser lançada - caso alguém decida desativar as afirmações posteriormente, isso fará com que ela falhe inofensivamente.

Outra coisa a considerar - quais são suas opções de recuperação? Às vezes você não tem um caminho de recuperação. O que eu acho que é o melhor exemplo disso é o primeiro lançamento do foguete Ariane V. Houve um erro não detectado / 0 (na verdade, um estouro de divisão) que resultou na destruição do booster. Na realidade, o código que travou não teve nenhum propósito naquele momento, tornou-se discutível no instante em que os reforçadores de correia acenderam. Depois que acendem sua órbita ou lança, você faz o melhor que pode, erros não podem ser permitidos. (Se o foguete se perder, devido a isso, o responsável pela segurança do alcance vira a chave.)

O que eu recomendo é usar uma assertdeclaração no seu final, em um desses dois estilos:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else {
        assert i > current
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    }
}

Ou uma declaração de código morto:

setCircle(circle, i, { current }) {
    if (i == current) {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
    } else if (i < current) {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
    } else if (i > current) {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
    } else {
        assert False, "Unreachable code"
    }
}

A ferramenta de cobertura de código geralmente pode ser configurada para ignorar código como "afirmar falso" no relatório de cobertura.

Ao colocar a condição em uma asserção, você documenta efetivamente a condição de uma ramificação explicitamente, mas, diferentemente de um comentário, a condição da asserção pode realmente ser verificada e falhará se você mantiver as asserções ativadas durante o desenvolvimento ou a produção (geralmente recomendo manter as asserções ativadas na produção se eles não afetam muito o desempenho).

Eu defini uma macro "afirmada" que avalia uma condição e, em uma compilação de depuração, cai no depurador.

Então, se eu tenho 100% de certeza de que uma das três condições deve ser verdadeira, escrevo

If condition 1 ...
Else if condition 2 .,,
Else if asserted (condition3) ...

Isso deixa claro o suficiente que uma condição será verdadeira e nenhuma ramificação extra para uma afirmação é necessária.

Eu recomendo evitar completamente o resto . Use ifpara declarar o que o bloco de código deve manipular e finalize o bloco saindo da função.

Isso resulta em um código muito claro:

setCircle(circle, i, { current })
{
    if (i == current)
    {
        circle.src = 'images/25CE.svg'
        circle.alt = 'Now picking'
        return
    }
    if (i < current)
    {
        circle.src = 'images/25C9.svg'
        circle.alt = 'Pick failed'
        return
    }
    if (i > current)
    {
        circle.src = 'images/25CB.svg'
        circle.alt = 'Pick chance'
        return
    }
    throw new Exception("Condition not handled.");
}

A final ifé obviamente redundante ... hoje. Pode se tornar um pensamento muito importante se / quando algum desenvolvedor futuro reorganizar os blocos. Portanto, é útil deixá-lo lá.