Avaliação de curto-circuito, é uma prática ruim?

Algo que eu conheço há algum tempo, mas nunca considerei, é que, na maioria dos idiomas, é possível dar prioridade aos operadores em uma instrução if com base em seu pedido. Costumo usar isso como uma maneira de evitar exceções de referência nula, por exemplo:

if (smartphone != null && smartphone.GetSignal() > 50)
{
   // Do stuff
}

Nesse caso, o código resultará primeiro na verificação de que o objeto não é nulo e, em seguida, no uso desse objeto, sabendo que ele existe. A linguagem é inteligente porque sabe que, se a primeira instrução for falsa, não faz sentido avaliar a segunda instrução e, portanto, nunca será lançada uma exceção de referência nula. Isso funciona da mesma para ande oroperadores.

Isso também pode ser útil em outras situações, como verificar se um índice está dentro dos limites de uma matriz e essa técnica pode ser executada em várias linguagens, pelo que sei: Java, C #, C ++, Python e Matlab.

Minha pergunta é: esse tipo de código é uma representação de más práticas? Essa má prática surge de algum problema técnico oculto (isto é, pode resultar em erro) ou leva a um problema de legibilidade para outros programadores? Poderia ser confuso?

Não, isso não é uma prática ruim. Confiar no curto-circuito de condicionais é uma técnica útil e amplamente aceita - desde que você esteja usando uma linguagem que garanta esse comportamento (que inclui a grande maioria das linguagens modernas).

Seu exemplo de código é bastante claro e, de fato, é frequentemente a melhor maneira de escrevê-lo. Alternativas (como ifinstruções aninhadas ) seriam mais confusas, mais complicadas e, portanto, mais difíceis de entender.

Algumas advertências:

Use o bom senso em relação à complexidade e legibilidade

A seguir, não é uma boa idéia:

if ((text_string != null && replace(text_string,"$")) || (text_string = get_new_string()) != null)

Como muitas maneiras "inteligentes" de reduzir as linhas no seu código, o excesso de confiança nessa técnica pode resultar em códigos difíceis de entender e propensos a erros.

Se você precisar lidar com erros, geralmente há uma maneira melhor

Seu exemplo é bom, desde que seja um estado normal do programa para o smartphone ser nulo. Se, no entanto, for um erro, você deverá incorporar um tratamento de erros mais inteligente.

Evite repetidas verificações da mesma condição

Como Neil aponta, muitas verificações repetidas da mesma variável em diferentes condicionais são evidências mais prováveis ​​de uma falha de design. Se você tem dez declarações diferentes polvilhadas através de seu código que não deve ser executado se smartphoneé null, considerar se há uma maneira melhor de lidar com isso do que verificar a variável de cada vez.

Não se trata especificamente de um curto-circuito; é um problema mais geral de código repetitivo. No entanto, vale a pena mencionar, porque é bastante comum ver código que possui muitas instruções repetidas como a sua instrução de exemplo.

Digamos que você estava usando uma linguagem de estilo C sem &&e precisava fazer o código equivalente, como na sua pergunta.

Seu código seria:

if(smartphone != null)
{
  if(smartphone.GetSignal() > 50)
  {
    // Do stuff
  }
}

Esse padrão apareceria muito.

Agora imagine a versão 2.0 de nossa linguagem hipotética introduzida &&. Pense em como seria legal!

&&é o meio idiomático reconhecido de fazer o que meu exemplo acima faz. Não é uma prática ruim usá-lo; de fato, é uma prática ruim não usá-lo em casos como o acima: um codificador experiente em uma linguagem desse tipo estaria se perguntando onde elseestava ou outro motivo para não fazer as coisas da maneira normal.

A linguagem é inteligente porque sabe que, se a primeira instrução for falsa, não faz sentido avaliar a segunda instrução e, portanto, nunca será lançada uma exceção de referência nula.

Não, você é esperto, porque sabia que se a primeira afirmação era falsa, não fazia sentido avaliar a segunda afirmação. A linguagem é burra como uma caixa de pedras e fez o que você pediu. (John McCarthy era ainda mais inteligente e percebeu que a avaliação de curto-circuito seria útil para as línguas).

A diferença entre você ser inteligente e o idioma ser inteligente é importante, porque as boas e más práticas geralmente se resumem a você ser tão inteligente quanto necessário, mas não mais inteligente.

Considerar:

if(smartphone != null && smartphone.GetSignal() > ((range = (range != null ? range : GetRange()) != null && range.Valid ? range.MinSignal : 50)

Isso estende seu código para verificar a range. Se o rangefor nulo, ele tenta defini-lo por uma chamada para, GetRange()embora isso possa falhar, rangeainda assim pode ser nulo. Se o intervalo não for nulo depois disso, e ela for usada, Validsua MinSignalpropriedade será usada, caso contrário, será usado um padrão 50.

Isso depende &&também, mas colocá-lo em uma linha provavelmente é muito inteligente (não tenho 100% de certeza de que estou certo e não vou checar novamente, porque esse fato demonstra meu argumento).

&&Porém, não é esse o problema aqui, mas a capacidade de usá-lo para colocar muito em uma expressão (uma coisa boa) aumenta nossa capacidade de escrever expressões difíceis de entender desnecessariamente (uma coisa ruim).

Além disso:

if(smartphone != null && (smartphone.GetSignal() == 2 || smartphone.GetSignal() == 5 || smartphone.GetSignal() == 8 || smartPhone.GetSignal() == 34))
{
  // do something
}

Aqui estou combinando o uso de &&com uma verificação de certos valores. Isso não é realista no caso de sinais telefônicos, mas ocorre em outros casos. Aqui, é um exemplo de eu não ser inteligente o suficiente. Se eu tivesse feito o seguinte:

if(smartphone != null)
{
  switch (smartphone.GetSignal())
  {
    case 2: case 5: case 8: case 34:
      // Do something
      break;
  }
}

Eu teria ganho em legibilidade e também em desempenho (as várias chamadas para GetSignal()provavelmente não teriam sido otimizadas).

Aqui, novamente, o problema não é apenas &&pegar esse martelo em particular e ver todo o resto como um prego; não usá-lo, deixe-me fazer algo melhor do que usá-lo.

Um caso final que se afasta das melhores práticas é:

if(a && b)
{
  //do something
}

Comparado com:

if(a & b)
{
  //do something
}

O argumento clássico de por que podemos favorecer o último é que há algum efeito colateral na avaliação de bque queremos se aé verdade ou não. Discordo disso: se esse efeito colateral é tão importante, faça-o acontecer em uma linha de código separada.

No entanto, em termos de eficiência, é provável que qualquer um dos dois seja melhor. O primeiro, obviamente, executará menos código (sem avaliar bem um único caminho de código), o que pode nos poupar quanto tempo for necessário para avaliar b.

O primeiro, porém, também tem mais um ramo. Considere se o reescrevemos em nossa hipotética linguagem de estilo C sem &&:

if(a)
{
  if(b)
  {
    // Do something
  }
}

Esse extra ifestá oculto em nosso uso de &&, mas ainda está lá. E, como tal, é um caso em que existe uma previsão de ramificação e, portanto, potencialmente uma má previsão de ramificação.

Por esse motivo, o if(a & b)código pode ser mais eficiente em alguns casos.

Aqui, eu diria que if(a && b)ainda é a melhor prática para começar: Mais comum, a única viável em alguns casos (em bque o erro aserá falso) e mais rápida do que nunca. Vale a pena notar que if(a & b)muitas vezes é uma otimização útil em alguns casos.

Você pode levar isso adiante e, em alguns idiomas, é a maneira idiomática de fazer as coisas: você também pode usar a avaliação de curto-circuito fora das declarações condicionais, e elas se tornam uma forma de declaração condicional. Por exemplo, no Perl, é idiomático que as funções retornem algo falso em caso de falha (e algo verdadeiro em caso de sucesso), e algo como

open($handle, "<", "filename.txt") or die "Couldn't open file!";

é idiomático. openretorna algo diferente de zero em caso de sucesso (para que a dieparte depois ornunca aconteça), mas indefinida em caso de falha e, em seguida, a chamada para dieacontecer (é a maneira de Perl criar uma exceção).

Isso é bom em idiomas onde todos fazem isso.

Embora eu concorde geralmente com a resposta de dan1111 , há um caso particularmente importante que não abrange: o caso em que smartphoneé usado simultaneamente. Nesse cenário, esse tipo de padrão é uma fonte bem conhecida de bugs difíceis de encontrar.

O problema é que a avaliação de curto-circuito não é atômica. Seu encadeamento pode verificar, ou smartphoneseja null, outro encadeamento e anulá-lo e, em seguida, o encadeamento tenta GetSignal()e explode prontamente .

Mas, claro, ele só faz isso quando as estrelas se alinham e tempo de seus segmentos é apenas direita.

Portanto, embora esse tipo de código seja muito comum e útil, é importante conhecer essa advertência para que você possa evitar com precisão esse bug desagradável.

Há muitas situações em que desejo verificar uma condição primeiro e só quero verificar uma segunda condição se a primeira condição tiver êxito. Às vezes, puramente por eficiência (porque não há sentido em verificar a segunda condição, se a primeira já falhar), outras vezes, caso contrário, meu programa falharia (sua verificação de NULL primeiro), outras vezes, caso contrário, os resultados seriam terríveis. (SE (desejo imprimir um documento) AND (falha na impressão de um documento)) ENTÃO exibe uma mensagem de erro - você não deseja imprimir o documento e verifique se a impressão falhou se não quisesse imprimir um documento no primeiro lugar!

Portanto, havia uma enorme necessidade de avaliação garantida de curto-circuito de expressões lógicas. Não estava presente em Pascal (que teve avaliação de curto-circuito não garantida), o que foi uma grande dor; Eu vi pela primeira vez em Ada e C.

Se você realmente acha que isso é "má prática", pegue qualquer código moderadamente complexo, reescreva-o para que ele funcione bem sem a avaliação de curto-circuito e volte e diga-nos como você gostou. É como se soubesse que a respiração produz dióxido de carbono e pergunta se a respiração é uma má prática. Tente sem ele por alguns minutos.

Uma consideração, não mencionada em outras respostas: às vezes, essas verificações podem sugerir uma possível refatoração para o padrão de design do Objeto Nulo . Por exemplo:

if (currentUser && currentUser.isAdministrator()) 
  doSomething();

Pode ser simplificado para ser apenas:

if (currentUser.isAdministrator())
  doSomething ();

Se currentUserfor padronizado para algum 'usuário anônimo' ou 'usuário nulo' com uma implementação de fallback se o usuário não estiver conectado.

Nem sempre um cheiro de código, mas algo a considerar.

Eu vou ser impopular e dizer Sim, é uma prática ruim. SE a funcionalidade do seu código depende dele para implementar lógica condicional.

ie

 if(quickLogicA && slowLogicB) {doSomething();}

é bom, mas

if(logicA && doSomething()) {andAlsoDoSomethingElse();}

é ruim, e certamente ninguém concordaria ?!

if(doSomething() || somethingElseIfItFailed() && anotherThingIfEitherWorked() & andAlwaysThisThing())
{/* do nothing */}

Raciocínio:

Seu código erros se ambos os lados são avaliados em vez de simplesmente não executar a lógica. Argumentou-se que isso não é um problema, o operador 'e' é definido em c # para que o significado seja claro. No entanto, afirmo que isso não é verdade.

Razão 1. Histórico

Basicamente, você está confiando (no que foi originalmente planejado) em uma otimização de compilador para fornecer funcionalidade ao seu código.

Embora em c # a especificação da linguagem garanta que o operador booleano 'e' faça um curto-circuito. Isso não se aplica a todos os idiomas e compiladores.

O wikipeda lista vários idiomas e sua opinião sobre curtos-circuitos http://en.wikipedia.org/wiki/Short-circuit_evaluation, como você pode, existem muitas abordagens. E alguns problemas extras que não abordarei aqui.

Em particular, FORTRAN, Pascal e Delphi possuem sinalizadores de compilador que alternam esse comportamento.

Portanto: você pode achar que seu código funciona quando compilado para lançamento, mas não para depuração ou com um compilador alternativo etc.

Razão 2. Diferenças de idioma

Como você pode ver na wikipedia, nem todos os idiomas adotam a mesma abordagem para curtos-circuitos, mesmo quando especificam como os operadores booleanos devem lidar com isso.

Em particular, o operador 'and' do VB.Net não provoca curto-circuito e o TSQL tem algumas peculiaridades.

Dado que uma 'solução' moderna provavelmente incluirá vários idiomas, como javascript, regex, xpath etc., você deve levar isso em consideração ao tornar clara a funcionalidade do seu código.

Razão 3. Diferenças dentro de um idioma

Por exemplo, o inline do VB se comporta de maneira diferente do seu if. Novamente, em aplicativos modernos, seu código pode se mover entre, por exemplo, code-behind e um formulário da web embutido, você precisa estar ciente das diferenças de significado.

Razão 4. Seu exemplo específico de verificação nula do parâmetro de uma função

Este é um exemplo muito bom. É algo que todo mundo faz, mas é realmente uma prática ruim quando você pensa sobre isso.

É muito comum ver esse tipo de verificação, pois reduz consideravelmente suas linhas de código. Duvido que alguém o traga uma revisão de código. (e, obviamente, pelos comentários e classificações, você pode ver que é popular!)

No entanto, ele falha nas práticas recomendadas por mais de um motivo!

1. É muito claro, de várias fontes, que a melhor prática é verificar a validade de seus parâmetros antes de usá-los e lançar uma exceção se eles forem inválidos. Seu snippet de código não mostra o código circundante, mas você provavelmente deveria estar fazendo algo como:

   if (smartphone == null)
   {
       //throw an exception
   }
   // perform functionality
   

2. Você está chamando uma função de dentro da instrução condicional. Embora o nome da sua função implique que ele simplesmente calcule um valor, ele pode ocultar efeitos colaterais. por exemplo

   Smartphone.GetSignal() { this.signal++; return this.signal; }
   
   else if (smartphone.GetSignal() < 1)
   {
       // Do stuff 
   }
   else if (smartphone.GetSignal() < 2)
   {
       // Do stuff 
   }
   

   as melhores práticas sugerem:

   var s = smartphone.GetSignal();
   if(s < 50)
   {
       //do something
   }
   

Se você aplicar essas práticas recomendadas ao seu código, poderá ver que sua oportunidade de obter um código mais curto por meio do uso da condicional oculta desaparece. A melhor prática é fazer algo , lidar com o caso em que o smartphone é nulo.

Eu acho que você descobrirá que esse também é o caso de outros usos comuns. Você deve se lembrar que também temos:

condicionais em linha

var s = smartphone!=null ? smartphone.GetSignal() : 0;

coalescência nula

var s = smartphoneConnectionStatus ?? "No Connection";

que fornecem funcionalidade semelhante de tamanho de código curto com mais clareza

inúmeros links para apoiar todos os meus pontos:

https://stackoverflow.com/questions/1158614/what-is-the-best-practice-in-case-one-argument-is-null

Validação de parâmetro do construtor em C # - Melhores práticas

Deve-se procurar nulo se ele não espera nulo?

https://msdn.microsoft.com/en-us/library/seyhszts%28v=vs.110%29.aspx

https://stackoverflow.com/questions/1445867/why-would-a-language-not-use-short-circuit-evaluation

http://orcharddojo.net/orchard-resources/Library/DevelopmentGuidelines/BestPractices/CSharp

exemplos de sinalizadores do compilador que afetam o curto-circuito:

Fortran: "sce | nosce Por padrão, o compilador executa avaliação de curto-circuito em expressões lógicas selecionadas usando regras XL Fortran. A especificação de sce permite que o compilador use regras não-XL Fortran. O compilador executará avaliação de curto-circuito se as regras atuais permitirem O padrão é nosce. "

Pascal: "B - Uma diretiva de flag que controla a avaliação de curto-circuito. Consulte Ordem de avaliação de operandos de operadores diádicos para obter mais informações sobre avaliação de curto-circuito. Consulte também a opção do compilador -sc do compilador de linha de comando para obter mais informações ( documentado no manual do usuário). "

Delphi: "O Delphi suporta a avaliação de curto-circuito por padrão. Ele pode ser desativado usando a diretiva de compilador {$ BOOLEVAL OFF}."