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

% explícito do blocoelse

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 - suponho - é ser explícito, mostrando que o desenvolvedor estava totalmente ciente de que a condição pode ser avaliada como false . Embora seja bom ser explícito, tal explicitação não deve ter o custo de três linhas extras de código .

Nem sequer mencionei o fato de que uma instrução if não é necessariamente seguida por else ou nada: ela pode ser seguida por um ou mais elif s também.

A presença da declaração return torna as coisas piores. Mesmo se você realmente tivesse código para executar dentro do bloco else , seria mais legível fazê-lo assim:

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

doOtherThings();
return somethingElse;

Isso faz com que o código use duas linhas a menos e desmembra o bloco else . As cláusulas Guard são sobre isso.

Observe, no entanto, que a técnica do seu colega poderia 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 sã de linha após o primeiro bloco if torna muito fácil interpretar incorretamente o código. No entanto, embora a regra de seu colega dificulte a leitura errada do código, uma solução mais fácil seria simplesmente adicionar uma nova linha.

Teste para true , não para false

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

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

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

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

A condição acima pode ser facilmente legível:

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

Em relação à primeira regra, este é um exemplo de digitação inútil. Não só demora mais para digitar, como também causa grandes confusões para quem lê o código. Se o código não for necessário, não o escreva. Isso se estenderia até mesmo a não ter um else preenchido no seu caso, pois o código retorna do bloco if :

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

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

Em relação ao segundo ponto, é bom evitar nomes booleanos que contenham um 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 um negativo novamente, como você aponta, leva a uma digitação mais inútil. O seguinte é muito mais claro do que ter um bloco if vazio:

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

1. Um argumento em favor das instruções else vazias.

Eu muitas vezes uso (e defendo) algo parecido com essa primeira construção, um vazio mais. Ele sinaliza para os leitores do código (ferramentas de análise humanas e automatizadas) que o programador pensou na situação. Faltando else declarações que deveriam estar presentes mataram pessoas, caíram veículos e custaram milhões de dólares. MISRA-C, por exemplo, ordena pelo menos um comentário dizendo que o final em falta é intencional em uma sequência if (condition_1) {do_this;} else if (condition_2) {do_that;} ... else if (condition_n) {do_something_else;} . Outros padrões de alta confiabilidade vão ainda mais longe: com poucas exceções, faltando, outras afirmações são proibidas.

Uma exceção é um comentário simples, algo como /* Else not required */ . Isso sinaliza a mesma intenção que as três linhas mais vazias. Outra exceção em que esse vazio não é necessário é onde é óbvio para os leitores do código e para as ferramentas de análise automatizadas que o vazio mais supérfluo. Por exemplo, if (condition) { do_stuff; return; } Da mesma forma, um vazio não é necessário no caso de throw something ou goto some_label ¹ em vez de return .

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

Este é um item de fatores humanos. A lógica booleana complexa tropeça muitas pessoas. Mesmo um programador experiente terá que pensar em 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 você deve preferir instruções then vazias.

A segunda seção diz "preferir" em vez de "tu deverás". É uma diretriz e não uma regra. A razão para essa diretriz preferir condições if positivas é que o código deve ser claro e óbvio. Uma cláusula vazia, em seguida, (por exemplo, if (condition) ; else do_something; ) viola isso. É uma programação ofuscada, fazendo com que até mesmo os mais experientes programadores façam backup e releem a condição if em sua forma negada. Então, escreva-o no formulário negado em primeiro lugar e omita o comando else (ou tenha um outro vazio ou um comentário para esse efeito se for obrigado a fazê-lo).

¹ Escrevi que as cláusulas que terminam com return , throw ou goto não exigem mais um vazio. É óbvio que a cláusula else não é necessária. Mas e quanto a goto ? Como um aparte, as regras de programação críticas para a segurança às vezes não permitem o retorno antecipado e quase sempre impedem o lançamento de exceções. No entanto, eles permitem goto em um formato restrito (por exemplo, goto cleanup1; ). Esse uso restrito de goto é a prática preferida em alguns lugares. O kernel do Linux, por exemplo, é chockfull de tais declarações goto .

Eu uso um branch else vazio (e às vezes um if-branch vazio) em casos muito raros: Quando é óbvio que a parte if e else deve ser tratada de alguma forma, mas por alguma razão não trivial o caso pode ser manipulado sem fazer nada. E, portanto, qualquer um que ler o código com a ação necessária imediatamente suspeitaria que algo está faltando e desperdiçará seu 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 mais sendo igual, prefira a brevidade.

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

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

Além disso, evite escrever um else branch se sair diretamente do if-branch.

Uma aplicação útil de explicitação seria colocar um comentário sempre que você passar por casos de comutação // FALLTHRU e usar um comentário ou um bloco vazio em que você precisa de uma instrução vazia for(a;b;c) /**/; .

Não existe uma regra rígida ou rápida sobre condições positivas ou negativas para uma instrução IF, não é do meu conhecimento. Eu pessoalmente prefiro codificar para um caso positivo em vez de negativo, quando aplicável. Eu certamente não farei isso, se isso me levar a fazer um bloco IF vazio, seguido por um ELSE cheio de lógica. Se tal situação surgisse, levaria cerca de 3 segundos para refatorar o teste para um caso positivo, de qualquer forma.

O que eu realmente não gosto nos seus exemplos é o espaço vertical completamente desnecessário ocupado pelo espaço 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 diria que o espaço vertical adicionado poderia diminuir a legibilidade.

% explícito do blocoelse

Eu não concordo com isso como uma declaração geral cobrindo todas as declarações if , mas há momentos em que adicionar um bloco else por hábito é uma coisa boa.

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

Se devemos fazer algo, faça aqui.

Coisas assim obviamente não precisam de uma parte else .

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

e, em alguns casos, insistir em adicionar um else pode induzir em erro.

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

é não igual a

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

embora seja funcionalmente o mesmo. Escrever o primeiro if com um else vazio pode levar você ao segundo resultado, que é desnecessariamente feio.

Se estivermos verificando um estado específico, geralmente é uma boa ideia adicionar um else vazio apenas para lembrá-lo de cobrir 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 só se aplicam quando você está escrevendo um novo código . IMHO As cláusulas else vazias devem ser removidas antes do check-in.

Teste para true , não para false

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

Mesmo com código como

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

é chocante para o leitor, mas tornando-se

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

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

Codifiquei em BCPL há muitos anos e nessa linguagem há uma cláusula IF e uma cláusula UNLESS para que você codifique de maneira muito mais legível como:

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

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

Meu processo pessoal

Geralmente, quando escrevo código novo , muitas vezes adiciono um bloco else vazio a uma instrução if apenas para lembrar que ainda não abordei essa eventualidade. Isso me ajuda a evitar a armadilha DFS e garante que, quando eu revisar o código, perceba que há mais a ser feito. No entanto, geralmente, adiciono um comentário TODO para acompanhar.

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

Eu acho que geralmente eu uso else raramente no meu código, pois pode indicar um cheiro de código.

Para o primeiro ponto, usei uma linguagem que forçou instruções IF a serem usadas dessa maneira (em Opal, a linguagem por trás de um scraper de tela para colocar um front end de GUI em 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 otimizasse essas cláusulas ELSE adicionais. Mas para o código ao vivo, não está adicionando nada (em desenvolvimento, pode ser um marcador útil para mais código).

Uma vez eu uso algo como estas cláusulas extras quando uso o processamento de tipo CASE / WHEN. Eu sempre adiciono uma cláusula padrão mesmo se ela estiver vazia. Este é o hábito de longo prazo de linguagens que irão errar se tal cláusula não for usada, e forçar um pensamento sobre se as coisas realmente deveriam passar.

Prática de mainframe antiga (por exemplo, PL / 1 e COBOL), foi aceito que as verificações negativas eram menos eficientes. Isso poderia explicar o segundo ponto, embora atualmente haja economias de eficiência muito 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 instrução IF tão simples.

Gostaria de secundar a maioria das respostas que esvaziam else blocos são praticamente sempre um desperdício prejudicial de tinta eletrônica. Não adicione isso a menos que você tenha uma boa razão para fazê-lo. Nesse caso, o bloco vazio não deve estar vazio, deve conter um comentário explicando por que ele está lá.

A questão sobre como 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 alguns outros blocos para operar quando não estiver definido. Dessa forma, se você aplicar uma regra de não negativas, você aplicará as instruções if() {} else {...} (com um bloco if vazio!) Ou criará um segundo booleano para cada booleano que contiver 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 em um nome booleano e expresse negação como um único ! . Uma declaração como if(!doorLocked) é perfeitamente clara, uma afirmação como if(!doorUnlocked) nos cérebros. O último tipo de expressão é o que você precisa evitar a todo custo, não a presença de uma única condição ! em if() .

Eu diria que é definitivamente uma prática ruim. Adicionando instruções else vai adicionar um monte de linhas sem sentido para o seu código que não fazem nada e tornam ainda mais difícil de ler.

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

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

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

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

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

Deixar a ramificação else 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. O mesmo que deixar de fora completamente. No entanto, quando você está programando iterativamente, eu estabeleço muito poucas razões para sempre ter um bloco else .

...an if statement should be followed by an else statement, whether there is something to put into it or not.

Eu discordo que 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 a pena ressaltar que raramente tenho um branch vazio. Isso ocorre porque normalmente eu registro que eu entrei no ramo vazio. Dessa forma, posso desenvolver apenas mensagens de log. Eu raramente uso um depurador. Em ambientes de produção, você não obtém um depurador; é bom poder solucionar problemas de produção com as mesmas ferramentas que você usa para desenvolver.

Secondly, it's better to test for true values rather than false. That means that it's better to test a 'doorClosed' variable instead of a '!doorOpened' variable

Eu tenho sentimentos mistos sobre isso. Uma desvantagem de ter doorClosed e doorOpened é o dobro do número de palavras / termos de que você precisa estar ciente. Outra desvantagem é que com o tempo o significado de doorClosed e doorOpened pode mudar (outros desenvolvedores entram depois de você) e você pode acabar com duas propriedades que não são mais negações precisas umas das outras. Em vez de evitar negações, eu valorizo a adaptação da linguagem do código (nomes de classes, nomes de variáveis, etc.) ao vocabulário dos usuários corporativos e aos requisitos que me são fornecidos. Eu não gostaria de inventar um termo totalmente novo para evitar um ! se esse termo tiver significado apenas para um desenvolvedor que os usuários corporativos não entenderão. Eu quero que os desenvolvedores falem a mesma linguagem 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, e não depois. O desenvolvimento deve lidar com esse problema antes de começar a codificação.

I am prone to doubt my instinct.

É bom continuar a se questionar.

Is it a good/bad/"doesn't matter" practice?

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

Is it common practice?

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

Eu só vou abordar a segunda parte da questão e isso vem do meu ponto de vista de trabalhar em sistemas industriais e manipular dispositivos no mundo real.

No entanto, isso é de algum modo um comentário extenso, em vez de uma resposta.

Quando é declarado

Secondly, it's better to test for true values rather than false. That means that it's better to test a 'doorClosed' variable instead of a '!doorOpened' variable

His argument is that it makes clearer what the code is doing.

Do meu ponto de vista, a suposição está sendo feita aqui de 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 está fechada.

Assim, dependendo de onde um único sensor digital binário está localizado, 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 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. Portanto, doorClosed e !doorOpened provavelmente não são sinônimos e qualquer tentativa de fingir que eles são sinônimos é um pensamento errôneo que pressupõe um conhecimento maior do estado do sistema do que realmente existe.

Voltando à sua pergunta, eu apoiaria o palavreado que corresponde à origem da informação que a variável representa. Assim, se a informação é derivada do lado fechado da porta, então vá com doorClosed etc. Isso pode implicar o uso de doorClosed ou !doorClosed conforme necessário em várias declarações, conforme necessário, resultando em confusão potencial com o uso do negação. Mas o que ele não faz é implicitamente propagar suposições sobre o estado do sistema.

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

Às vezes eu só preciso saber se a porta está fechada ou não fechada. 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 erros contra a porta sendo aberta e fechada simultaneamente) ou haveria um sensor analógico medindo como a porta estava "aberta".

Um exemplo do primeiro seria a porta de um 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 como a porta está aberta.

Um exemplo do segundo seria um simples acionador motorizado. Você inibe a condução do motor para frente quando o atuador está totalmente fora. E você inibe a condução do motor em marcha a ré quando o atuador está totalmente em operação.

Fundamentalmente, o número e o tipo de sensores se resume a executar 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.

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

void foo()
{
    if (condition) return;

    doStuff();
    ...
}

Este é 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 que os futuros programadores se perguntem se um no-op foi intencional ou não. Ele também pode melhorar a legibilidade, pois você pode ser obrigado a fazer uma nova função (dividindo, assim, grandes funções em funções individuais menores).

Regras de estilo concreto são sempre ruins. As diretrizes são boas, mas no final há casos em que você pode esclarecer as coisas fazendo algo que não segue as diretrizes.

Dito isso, há muito ódio pelo vazio, mais "desperdiça linhas", "digitação extra", que são ruins. Você poderia argumentar a favor de mover os colchetes para a mesma linha se realmente precisar do espaço vertical, mas se isso for um problema, você não deveria estar colocando seu {em uma linha separada de qualquer maneira.

Como mencionado anteriormente, 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 else ao escrever o if, embora, como o @OldCurmudgeon menciona, há realmente dois casos de uso diferentes. Um deveria ter outro, não deveria. Infelizmente, não é algo que você possa sempre ver de relance, muito menos com um linter, daí a dogmática "sempre coloque um bloco a mais".

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

static var cached = null

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

    return cached
}

se a lógica real (mental / inglesa) envolver um negativo, o mesmo aconteceria com a instrução if.