O desenvolvedor insiste se as instruções não devem ter condições negadas e devem sempre ter um bloco else

Eu tenho um conhecido, um desenvolvedor mais experiente do que eu. Estávamos conversando sobre práticas de programação e fiquei surpreso com a abordagem dele sobre as declarações 'if'. Ele insiste em algumas práticas relacionadas a declarações if que eu acho bastante estranhas.

Em primeiro lugar , uma declaração if deve ser seguida por uma declaração else, se há algo a ser inserido ou não. O que leva ao código assim:

if(condition) 
{
    doStuff();
    return whatever;
}
else
{
}

Em segundo lugar , é melhor testar os valores verdadeiros do que falsos. Isso significa que é melhor testar uma variável 'doorClosed' em vez de uma variável '! DoorOpened'

Seu argumento é que isso torna mais claro o que o código está fazendo.

O que me confunde bastante, pois uma combinação dessas duas regras pode levá-lo a escrever esse tipo de código se ele quiser fazer algo quando a condição não for atendida.

if(condition)
{
}
else
{
    doStuff();
    return whatever;
}

Meu sentimento sobre isso é que é realmente muito feio e / ou que a melhoria da qualidade, se houver, é insignificante. Mas, quando jovem, sou propenso a duvidar do meu instinto.

Então, minhas perguntas são: É uma prática boa / ruim / "não importa"? É prática comum?

elseBloco explícito

A primeira regra apenas polui o código e o torna nem mais legível nem menos propenso a erros. O objetivo do seu colega - eu suponho - é ser explícito, mostrando que o desenvolvedor estava plenamente ciente de que a condição pode ser avaliada false. Embora seja bom ser explícito, essa explicitação não deve custar três linhas de código extras .

Nem mencionei o fato de que uma ifdeclaração não é necessariamente seguida por um elseou nada: poderia ser seguida por um ou mais elifs também.

A presença da returndeclaração piora as coisas. Mesmo se você realmente tivesse código para executar dentro do elsebloco, seria mais legível fazer isso da seguinte maneira:

if (something)
{
    doStuff();
    return whatever;
}

doOtherThings();
return somethingElse;

Isso faz com que o código tome duas linhas a menos e desinere o elsebloco. As cláusulas de guarda são sobre isso.

Observe, no entanto, que a técnica do seu colega pode resolver parcialmente um padrão muito desagradável de blocos condicionais empilhados sem espaços:

if (something)
{
}
if (other)
{
}
else
{
}

No código anterior, a falta de uma quebra de linha sã após o primeiro ifbloco facilita muito a interpretação incorreta do código. No entanto, embora a regra do seu colega dificulte a leitura incorreta do código, uma solução mais fácil seria simplesmente adicionar uma nova linha.

Teste para true, não parafalse

A segunda regra pode fazer algum sentido, mas não em sua forma atual.

Não é falso que testar uma porta fechada seja mais intuitivo do que testar uma porta não aberta . Negações, e especialmente aninhadas, geralmente são difíceis de entender:

if (!this.IsMaster || (!this.Ready && !this.CanWrite))

Para resolver isso, em vez de adicionar blocos vazios, crie propriedades adicionais, quando relevantes, ou variáveis ​​locais .

A condição acima pode ser tornada legível com bastante facilidade:

if (this.IsSlave || (this.Synchronizing && this.ReadOnly))

Em relação à primeira regra, este é um exemplo de digitação inútil. Não apenas demora mais para digitar, como também causa muita confusão para quem lê o código. Se o código não for necessário, não o escreva. Isso pode até estender a não ser preenchida elseno seu caso, pois o código retorna do ifbloco:

if(condition) 
{
    doStuff();
    return whatever;
}

doSomethingElse(); // no else needed
return somethingelse;

Em relação ao segundo ponto, é bom evitar nomes booleanos que contenham um valor negativo:

bool doorNotOpen = false; // a double negative is hard to reason
bool doorClosed = false; // this is much clearer

No entanto, estender isso para não testar novamente um negativo, como você aponta, leva a uma digitação mais inútil. O seguinte é muito mais claro do que ter um ifbloco vazio :

if(!condition)
{
    doStuff();
    return whatever;
}

1. Um argumento a favor de elsedeclarações vazias .

Muitas vezes uso (e defendo) algo semelhante a essa primeira construção, uma outra coisa vazia. Ele sinaliza aos leitores do código (ferramentas de análise humana e automatizada) que o programador pensou um pouco na situação. As elsedeclarações ausentes que deveriam estar presentes mataram pessoas, bateram em veículos e custaram milhões de dólares. O MISRA-C, por exemplo, exige pelo menos um comentário dizendo que o final ausente é intencional em uma if (condition_1) {do_this;} else if (condition_2) {do_that;} ... else if (condition_n) {do_something_else;}sequência. Outros altos padrões de confiabilidade vão ainda mais longe: com algumas exceções, as instruções else else são proibidas.

Uma exceção é um comentário simples, algo ao longo das linhas de /* Else not required */. Isso indica a mesma intenção que as três linhas se esvaziam. Outra exceção em que esse mais vazio não é necessário é o óbvio tanto para os leitores do código quanto para as ferramentas de análise automatizada que esvaziam o excesso. Por exemplo, da if (condition) { do_stuff; return; }mesma forma, um outro vazio não é necessário no caso de throw somethingou goto some_label¹ no lugar de return.

2. Um argumento para preferir if (condition)sobre if (!condition).

Este é um item de fatores humanos. A lógica booleana complexa leva muitas pessoas. Mesmo um programador experiente terá que pensar if (!(complex || (boolean && condition))) do_that; else do_this;. No mínimo, reescreva isso como if (complex || (boolean && condition)) do_this; else do_that;.

3. Isso não significa que se deva preferir thendeclarações vazias .

A segunda seção diz "prefira" ao invés de "tu". É uma diretriz e não uma regra. A razão para essa diretriz preferir ifcondições positivas é que o código deve ser claro e óbvio. Uma cláusula vazia (por exemplo if (condition) ; else do_something;) viola isso. É uma programação ofuscada, fazendo com que até mesmo os programadores mais experientes iffaçam backup e reler a condição em sua forma negada. Portanto, escreva-o na forma negada em primeiro lugar e omita a instrução else (ou coloque um else em branco ou comente nesse sentido, se for solicitado a fazê-lo).

¹ Eu escrevi que, em seguida, cláusulas que terminam com return, throwou gotonão exigem um outro vazio. É óbvio que a cláusula else não é necessária. Mas que tal goto? Como um aparte, as regras de programação críticas à segurança às vezes proíbem o retorno antecipado e quase sempre proíbem exceções de lançamento. No entanto, eles permitem de gotoforma restrita (por exemplo, goto cleanup1;). Esse uso restrito gotoé a prática preferida em alguns lugares. O kernel do Linux, por exemplo, está repleto de tais gotodeclarações.

Eu uso um ramo else vazio (e algumas vezes um ramo if vazio) em casos muito raros: quando é óbvio que a parte if e else deve ser tratada de alguma forma, mas por algum motivo não trivial, o caso pode ser tratado por fazendo nada. E, portanto, quem lê o código com a ação else necessária suspeita imediatamente que algo está faltando e perde tempo.

if (condition) {
    // No action needed because ...
} else {
    do_else_action()
}

if (condition) {
    do_if_action()
} else {
    // No action needed because ...
}

Mas não:

if (condition) {
    do_if_action()
} else {
    // I was told that an if always should have an else ...
}

Tudo o resto é igual, prefere concisão.

O que você não escreve, ninguém precisa ler e entender.

Embora ser explícito possa ser útil, esse é apenas o caso se tornar óbvio, sem verbosidade indevida, que o que você escreveu é realmente o que você queria escrever.

Portanto, evite galhos vazios, eles não são apenas inúteis, mas também incomuns e, portanto, levam à confusão.

Além disso, evite escrever uma ramificação else se sair diretamente da ramificação if.

Uma aplicação útil da explicitação seria colocar um comentário sempre que você passar por casos de interrupção // FALLTHRUe usar um comentário ou um bloco vazio onde você precisa de uma declaração vazia for(a;b;c) /**/;.

Não existe uma regra rígida e rápida sobre condições positivas ou negativas para uma declaração IF, que eu não saiba. Pessoalmente, prefiro codificar para um caso positivo do que negativo, quando aplicável. Certamente não o farei, se isso me levar a criar um bloco IF vazio, seguido por um ELSE cheio de lógica. Se tal situação surgisse, levaria 3 segundos para refatorá-la de volta aos testes de um caso positivo.

O que eu realmente não gosto nos seus exemplos, porém, é o espaço vertical completamente desnecessário ocupado pelo ELSE em branco. Simplesmente não há razão alguma para fazer isso. Ele não adiciona nada à lógica, não ajuda a documentar o que o código está fazendo e não aumenta a legibilidade. Na verdade, eu argumentaria que o espaço vertical adicionado poderia diminuir a legibilidade.

elseBloco explícito

Eu discordo disso como uma declaração geral que abrange todas as ifdeclarações, mas há momentos em que adicionar um elsebloqueio ao hábito é uma coisa boa.

Uma ifafirmação, na minha opinião, abrange duas funções distintas.

Se devemos fazer algo, faça-o aqui.

Obviamente, coisas assim não precisam de uma elseparte.

    if (customer.hasCataracts()) {
        appointmentSuggestions.add(new CataractAppointment(customer));
    }
    if (customer.isDiabetic()) {
        customer.assignNurse(DiabeticNurses.pickBestFor(customer));
    }

e, em alguns casos, insistir em adicionar um elsepode enganar.

    if (k > n) {
        return BigInteger.ZERO;
    }
    if (k <= 0 || k == n) {
        return BigInteger.ONE;
    }

não é o mesmo que

    if (k > n) {
        return BigInteger.ZERO;
    } else {
        if (k <= 0 || k == n) {
            return BigInteger.ONE;
        }
    }

mesmo que seja funcionalmente o mesmo. Escrever o primeiro ifcom um vazio elsepode levar ao segundo resultado desnecessariamente feio.

Se estamos verificando um estado específico, geralmente é uma boa ideia adicionar um vazio elseapenas para lembrá-lo de que essa eventualidade

        // Count wins/losses.
        if (doors[firstChoice] == Prize.Car) {
            // We would have won without switching!
            winWhenNotSwitched += 1;
        } else {
            // We win if we switched to the car!
            if (doors[secondChoice] == Prize.Car) {
                // We picked right!
                winWhenSwitched += 1;
            } else {
                // Bad choice.
                lost += 1;
            }
        }

Lembre-se de que essas regras se aplicam somente quando você está escrevendo um novo código . IMHO As elsecláusulas vazias devem ser removidas antes do check-in.

Teste para true, não parafalse

Novamente, este é um bom conselho em nível geral, mas em muitos casos isso torna o código desnecessariamente complexo e menos legível.

Mesmo código como

    if(!customer.canBuyAlcohol()) {
        // ...
    }

é chocante para o leitor, mas torná-lo

    if(customer.canBuyAlcohol()) {
        // Do nothing.
    } else {
        // ...
    }

é pelo menos tão ruim, se não pior.

Eu codifiquei no BCPL há muitos anos e nesse idioma há uma IFcláusula e uma UNLESScláusula para que você possa codificar muito mais facilmente como:

    unless(customer.canBuyAlcohol()) {
        // ...
    }

o que é significativamente melhor, mas ainda não é perfeito.

Meu processo pessoal

Geralmente, quando estou escrevendo um novo código, geralmente adiciono um elsebloco vazio a uma ifdeclaração apenas para lembrar que ainda não cobri essa eventualidade. Isso me ajuda a evitar a DFSarmadilha e garante que, ao revisar o código, observe que há mais a fazer. No entanto, costumo adicionar um TODOcomentário para acompanhar.

  if (returnVal == JFileChooser.APPROVE_OPTION) {
    handleFileChosen();
  } else {
    // TODO: Handle case where they pressed Cancel.
  }

Acho que geralmente uso elseraramente no meu código, pois geralmente pode indicar um cheiro de código.

Para o primeiro ponto, usei um idioma que forçou as instruções IF a serem usadas dessa maneira (no Opal, o idioma por trás de um raspador de tela de mainframe para colocar um front end da GUI nos sistemas de mainframe) e com apenas uma linha para o IF e o ELSE. Não foi uma experiência agradável!

Eu esperaria que qualquer compilador otimize essas cláusulas ELSE adicionais. Mas, para o código ativo, ele não está adicionando nada (no desenvolvimento, pode ser um marcador útil para código adicional).

Uma vez que eu uso algo parecido com essas cláusulas extras é ao usar o processamento de tipo CASE / WHEN. Eu sempre adiciono uma cláusula padrão, mesmo que esteja vazia. Esse é um hábito a longo prazo das linguagens que errarão se tal cláusula não for usada e forçam a idéia de se as coisas realmente devem simplesmente desaparecer.

Há muito tempo, o mainframe (por exemplo, PL / 1 e COBOL) praticava que as verificações negativas eram menos eficientes. Isso poderia explicar o segundo ponto, embora atualmente haja economias de eficiência massivamente mais importantes que são ignoradas como micro otimizações.

A lógica negativa tende a ser menos legível, embora não tanto em uma declaração IF tão simples.

Eu diria que a maioria das respostas é que elseblocos vazios são quase sempre um desperdício prejudicial de tinta eletrônica. Não os adicione, a menos que você tenha um bom motivo para fazê-lo; nesse caso, o bloco vazio não deve estar vazio, deve conter um comentário explicando por que está lá.

O problema de evitar negativos merece mais atenção: normalmente, sempre que você precisar usar um valor booleano, precisará de alguns blocos de código para operar quando estiver definido e de outros blocos para operar quando não estiver definido. Dessa forma, se você aplicar uma regra de não-negativos, aplica-se a ter if() {} else {...}instruções (com um ifbloco vazio !) Ou cria um segundo booleano para cada booleano que contém seu valor negado. Ambas as opções são ruins, pois confundem seus leitores.

Uma política útil é a seguinte: nunca use um formulário negado no nome de um booleano e expresse a negação como um único !. Uma afirmação como if(!doorLocked)é perfeitamente clara, uma afirmação como if(!doorUnlocked)nós cérebros. O tipo de expressão posterior é o que você precisa evitar a todo custo, não a presença de uma única !em uma if()condição.

Eu diria que é definitivamente uma prática ruim. Adicionar instruções else adicionará um monte de linhas inúteis ao seu código que não fazem nada e dificultam ainda mais a leitura.

Há outro padrão que ainda não foi mencionado sobre como lidar com o segundo caso que possui um if-block vazio. Uma maneira de lidar com isso seria retornar no bloco if. Como isso:

void foo()
{
    if (condition) return;

    doStuff();
    ...
}

Esse é um padrão comum para verificar condições de erro. O caso afirmativo ainda é usado, e o interior dele não está mais vazio, deixando os futuros programadores se perguntarem se um não-operacional era intencional ou não. Também pode melhorar a legibilidade, pois pode ser necessário que você crie uma nova função (dividindo, assim, funções grandes em funções individuais menores).

Há um ponto ao considerar o argumento "sempre tenha uma cláusula else" que eu não vi em nenhuma outra resposta: ele pode fazer sentido em um estilo de programação funcional. Tipo de.

Em um estilo de programação funcional, você lida com expressões em vez de instruções. Portanto, todo bloco de código tem um valor de retorno - incluindo uma if-then-elseexpressão. Isso impediria, no entanto, um elsebloco vazio . Deixe-me dar um exemplo:

var even = if (n % 2 == 0) {
  return "even";
} else {
  return "odd";
}

Agora, em linguagens com sintaxe inspirada no estilo C ou no estilo C (como Java, C # e JavaScript, só para citar alguns), isso parece estranho. No entanto, parece muito mais familiar quando escrito como tal:

var even = (n % 2 == 0) ? "even" : "odd";

Deixar a elseramificação vazia aqui faria com que um valor fosse indefinido - na maioria dos casos, não o que queremos ser um caso válido ao programar a funcionalidade. Mesmo com deixá-lo de fora completamente. No entanto, quando você está programando iterativamente, defino muito pouco motivo para sempre ter um elsebloco.

... uma declaração if deve ser seguida por uma declaração else, se há algo para colocar nela ou não.

Eu discordo if (a) { } else { b(); }deve ser reescrito como if (!a) { b(); }e if (a) { b(); } else { }deve ser reescrito como if (a) { b(); }.

No entanto, vale ressaltar que raramente tenho um ramo vazio. Isso ocorre porque normalmente eu registro que entrei no ramo vazio. Dessa forma, eu posso desenvolver mensagens totalmente fora do log. Eu raramente uso um depurador. Em ambientes de produção, você não recebe um depurador; é bom poder solucionar problemas de produção com as mesmas ferramentas que você usa para desenvolver.

Em segundo lugar, é melhor testar os valores verdadeiros do que falsos. Isso significa que é melhor testar uma variável 'doorClosed' em vez de uma variável '! DoorOpened'

Eu tenho sentimentos mistos sobre isso. Uma desvantagem de ter doorClosede doorOpenedé potencialmente dobra o número de palavras / termos dos quais você precisa estar ciente. Outra desvantagem é que, com o tempo, o significado doorClosede doorOpenedpode mudar (outros desenvolvedores aparecem depois de você) e você pode acabar com duas propriedades que não são mais negações precisas uma da outra. Em vez de evitar negações, valorizo ​​adaptar o idioma do código (nomes de classes, nomes de variáveis ​​etc.) ao vocabulário dos usuários de negócios e aos requisitos que me são dados. Eu não gostaria de inventar um termo totalmente novo para evitar uma!se esse termo tiver apenas significado para um desenvolvedor que os usuários comerciais não entenderão. Quero que os desenvolvedores falem o mesmo idioma que os usuários e as pessoas que escrevem os requisitos. Agora, se novos termos simplificam o modelo, isso é importante, mas isso deve ser tratado antes que os requisitos sejam finalizados, não depois. O desenvolvimento deve lidar com esse problema antes que eles comecem a codificar.

Estou propenso a duvidar do meu instinto.

É bom continuar a se questionar.

É uma prática boa / ruim / "não importa"?

Até certo ponto, muito disso não importa. Revise seu código e adapte-se à sua equipe. Isso geralmente é a coisa certa a se fazer. Se todos na sua equipe quiserem codificar de uma certa maneira, provavelmente é melhor fazê-lo dessa maneira (mudar uma pessoa - você - leva menos pontos da história do que mudar um grupo de pessoas).

É prática comum?

Não me lembro de ter visto um galho vazio. Eu vejo pessoas adicionando propriedades para evitar negações, no entanto.

Vou abordar apenas a segunda parte da pergunta e isso vem do meu ponto de vista de trabalho em sistemas industriais e manipulação de dispositivos no mundo real.

No entanto, este é de algum modo um comentário estendido, e não uma resposta.

Quando é declarado

Em segundo lugar, é melhor testar os valores verdadeiros do que falsos. Isso significa que é melhor testar uma variável 'doorClosed' em vez de uma variável '! DoorOpened'

Seu argumento é que isso torna mais claro o que o código está fazendo.

Do meu ponto de vista, assume-se aqui que o estado da porta é um estado binário quando, na verdade, é pelo menos um estado ternário:

1. A porta está aberta.
2. A porta não está totalmente aberta nem totalmente fechada.
3. A porta esta fechada.

Portanto, dependendo da localização de um único sensor digital binário, você pode supor apenas um dos dois conceitos:

1. Se o sensor estiver no lado aberto da porta: A porta está aberta ou não está aberta
2. Se o sensor estiver no lado fechado da porta: A porta está fechada ou não fechada.

E você não pode trocar esses conceitos através do uso de uma negação binária. Assim, doorClosede !doorOpenedprovavelmente não são sinônimos, e qualquer tentativa de fingir que são sinônimos é um pensamento errôneo que pressupõe um maior conhecimento do estado do sistema do que realmente existe.

Voltando à sua pergunta, eu apoiaria verborragia que corresponde à origem das informações que a variável representa. Portanto, se a informação for derivada do lado fechado da porta, vá com doorClosedetc. Isso pode implicar o uso de uma doorClosedou !doorClosedconforme necessário em várias declarações, conforme necessário, resultando em possível confusão com o uso da negação. Mas o que isso não faz é propagar implicitamente suposições sobre o estado do sistema.

Para fins de discussão do trabalho que realizo, a quantidade de informações que tenho disponíveis sobre o estado do sistema depende da funcionalidade exigida pelo próprio sistema geral.

Às vezes, só preciso saber se a porta está fechada ou não. Nesse caso, apenas um único sensor binário será suficiente. Mas, em outros momentos, preciso saber que a porta está aberta, fechada ou em transição. Nesses casos, haveria dois sensores binários (em cada extremo do movimento da porta - com verificação de erro contra a porta sendo aberta e fechada simultaneamente) ou haveria um sensor analógico medindo o quão 'aberta' a porta estava.

Um exemplo do primeiro seria a porta do forno de microondas. Você ativa a operação do forno com base na porta sendo fechada ou não fechada. Você não se importa com a abertura da porta.

Um exemplo do segundo seria um simples atuador acionado por motor. Você inibe a condução do motor para a frente quando o atuador está totalmente desligado. E você inibe a condução do motor em marcha à ré quando o atuador está totalmente inserido.

Fundamentalmente, o número e o tipo de sensores se resumem à execução de uma análise de custo dos sensores em relação a uma análise de requisitos do que é necessário para alcançar a funcionalidade necessária.

Regras de estilo concretas são sempre ruins. As diretrizes são boas, mas no final existem casos em que você pode deixar as coisas mais claras, fazendo algo que não segue completamente as diretrizes.

Dito isto, há muito ódio pelo mais vazio "desperdiça linhas" "digitação extra", que é ruim. Você pode argumentar sobre como mover os colchetes para a mesma linha se realmente precisar do espaço vertical, mas se esse for um problema, você não deve colocar seu {em uma linha separada de qualquer maneira.

Como mencionado em outro lugar, ter o bloco else é muito útil para mostrar que você deseja explicitamente que nada aconteça no outro caso. Depois de fazer muita programação funcional (onde mais é necessário), aprendi que você deve sempre considerar o resto ao escrever o if, embora como o @OldCurmudgeon mencione, na verdade existem dois casos de uso diferentes. Um deveria ter outro, não deveria. Infelizmente, não é algo que você possa sempre dizer de relance, muito menos com um ponteiro, daí a dogmática "sempre coloque um outro bloco".

Quanto aos 'não negativos', novamente, regras absolutas são ruins. Ter um vazio se pode ser estranho, especialmente se for do tipo que não precisa de mais nada, então escreva tudo isso em vez de um! ou um == false é ruim. Dito isto, há muitos casos em que o negativo faz sentido. Um exemplo comum seria armazenar em cache um valor:

static var cached = null

func getCached() {
    if !cached {
        cached = (some calculation, etc)
    }

    return cached
}

se a lógica real (mental / inglesa) envolve uma negativa, a declaração if deve também.