Pergunta

Durante uma revisão de código, um dev sênior comentou sobre alguns nidificação eu tinha acontecendo no meu código. Ele sugeriu que eu definir um valor booleano de modo que eu nunca ter mais de um nível de aninhamento. Eu acho que meu código é mais legível, mas deseja obter a opinião dos outros devs sobre este assunto. Que é melhor estilo? É sua aversão instintiva para a desova fundada?

A seguir estão alguns exemplos de código simplificado.

Nested:

If(condition1)
{
    If(condition2)
    {
        if(condition3)
        {
            return true;
        }
        else
        {
            log("condition3 failed");
        }
    else
    {
        log("condition2 failed")
    }
}
else
{
    log("condition1 failed")
}

return false;

ou

Bool Driven:

bool bRC = false;

bRC = (condition1);
if(brc)
{
    bRC = (condition2);
}
else
{
    log("condition1 failed");
    return false;
}

if(bRC)
{
    bRC = (condition3);
}
else
{
    log("condition2 failed");
    return false;
}

if(bRC)
{
    return true;
}
else
{
    log("condition3 failed");
    return false;
}
Foi útil?

Solução

I como o seu melhor, mas eu provavelmente faria algo como:

if (condition1 && condition2 && condition3)
{
    return true;
}
else if (!condition1)
{
    log("condition1 failed");
}
else if (!condition2)
{
    log("condition2 failed");
}
else
{
    log("condition3 failed");
}
return false;

Se as condições são expressões complexas, então eu poderia atribuir as expressões a variáveis ??apropriadamente nomeados antes de avaliar o caso declarações para evitar ter que recalcular os valores em cada caso.

Esta é assumindo que o modo normal é que todas as condições são verdadeiras e, portanto, você quer ter que verificar primeiro. Se o modo normal é que uma ou mais condições são falsas, então eu reordenar-lo e verificar cada negação por sua vez e simplesmente retornar true se todas as verificações falhou. Isso também eliminaria a necessidade de variáveis ??temporárias para tomar o lugar de expressões complexas.

Outras dicas

Se você não tem quaisquer regras tolas sobre vários pontos de retorno, eu acho que isso é muito bom (e o mesmo acontece com outra pessoa, mas eles apagaram sua resposta por razões desconhecidas):

if(!condition1)
{
    log("condition1 failed");
    return false;
}

if(!condition2)
{
    log("condition2 failed");
    return false;
}

if(!condition3)
{
    log("condition3 failed");
    return false;
}

return true;

Talvez esta seja uma aversão instintiva igual a super-nidificação, mas certamente é mais limpo do que seu porcaria armazenar as condições booleanas em determinados valores. No entanto, pode ser menos legível no contexto: considerar se uma das condições era isreadable(). É claro que dizer if(isreadable()) porque queremos saber se algo é legível. if(!isreadable()) sugere, se queremos saber se não é legível, o que não é a nossa intenção. É certamente discutível que pode haver situações em que um é mais legível / intuitiva do que o outro, mas eu sou um fã desta maneira eu mesmo. Se alguém se levanta pendurado na volta, você poderia fazer isso:

if(!condition1)
    log("condition1 failed");

else if(!condition2)
    log("condition2 failed");

else if(!condition3)
    log("condition3 failed");

else
    return true;

return false;

Mas isso é menos clara, e menos "claro" na minha opinião.

Eu, pessoalmente encontrar o código aninhada significativamente mais fácil de ler.

I geralmente preferem aninhados para as minhas condições; Claro que, se minhas condições aninhadas estão a ficar recuado demasiado longe para a direita, eu tenho que começar a se perguntar se há uma maneira melhor de fazer o que eu estou tentando fazer (refatoração, redesenhando, etc ..)

Semelhante à versão aninhada, mas muito mais limpo para mim:

if not condition1:
    log("condition 1 failed")
else if not condition2:
    log("condition 2 failed")
else if not condition3:
    log("condition 3 failed")
else
    return true;
return false;

Tenha em atenção que cada condição é avaliada uma vez.

O segundo estilo é absurdamente detalhado: ele realmente sugerem exatamente isso? Você não precisa mais dessas declarações if, porque há uma return na parte mais.

Com o exemplo aninhada você está confiando em não esquecendo de incluir quaisquer cláusulas possível else.

Nenhum destes parece satisfatório para mim.

A forma impulsionado bool é confuso. Nidificação é bom, se necessário, mas você pode remover alguns da profundidade de assentamento, combinando as condições em uma instrução, ou chamar um método onde alguns dos mais avaliação é feita.

Eu acho que ambas as formas são possíveis e têm os seus prós e contras. Gostaria de usar o estilo-driven bool nos casos em que eu teria de aninhamento muito profundo (8 ou 10 ou algo parecido). No seu caso com 3 níveis, eu iria escolher o seu estilo, mas para a amostra exata de cima, eu iria assim:

void YourMethod(...)
{
  if (condition1 && condition2 && consition3)
    return true;

  if (!condition 1)
    log("condition 1 failed");

  if (!condition 2)
    log("condition 2 failed");

  if (!condition 3)
    log("condition 3 failed");

  return result;
}

... ou assim se preferir um único ponto de saída (como eu faço) ...

void YourMethod(...)
{
  bool result = false;

  if (condition1 && condition2 && consition3)
  { 
    result = true;
  }
  else
  {
    if (!condition 1)
      log("condition 1 failed");

    if (!condition 2)
      log("condition 2 failed");

    if (!condition 3)
      log("condition 3 failed");
  }
  return result;
}

Dessa forma, você vai ter toda a condição falhou registrado na primeira corrida. No seu exemplo, você vai ter apenas uma condição falhou registrada mesmo se houver mais de um condições de falha.

Eu provavelmente iria com

   if (!condition1)      log("condition 1 failed");
   else if (!condition2) log("condition 2 failed");
   else if (!condition3) log("condition 3 failed");
   // -------------------------------------------
   return condition1 && condition2 && condition3;

que eu acredito é equivilent e muito mais limpo ...

Além disso, uma vez que o cliente decide que todas as condições devem ser avaliados e registrados se eles falharem, não apenas o primeiro que falhar, isso é muito mais fácil de modificar para fazer isso:

   if (!condition1) log("condition 1 failed");
   if (!condition2) log("condition 2 failed");
   if (!condition3) log("condition 3 failed");
   // -------------------------------------------
   return condition1 && condition2 && condition3;

Isto é como eu iria implementá-lo, desde que suas implementações realmente refletir o comportamento desejado.

if (!condition1) {
    log("condition1 failed");
    return false;
}
if (!condition2) {
    log("condition2 failed");
    return false;
}
if (!condition3) {
    log("condition3 failed");
    return false;
}
return true;

No entanto, eu acho que é mais provável que todas as condições não devem ser registrados.

result = true;
if (!condition1) {
    log("condition1 failed");
    result = false;
}
if (!condition2) {
    log("condition2 failed");
    result = false;
}
if (!condition3) {
    log("condition3 failed");
    result = false;
}
return result;

Se a manipulação suportes de linguagem exceção, eu iria com o seguinte:

try {
    if (!condition1) {
        throw "condition1 failed";
    }

    if (!condition2) {
        throw "condition2 failed";
    }

    if (!condition3) {
        throw "condition3 failed";
    }

    return true;

} catch (e) {
    log(e);
    return false;
}

EDIT de Charles Bretana: Por favor, veja Usando exceções para controle de fluxo

Eu não gosto de qualquer maneira. Quando você tem tanto ninhos algo está errado. No caso de uma validação de formulário ou algo que de fato requerem algo assim tentar descobrir algo que é mais modular ou compacta.

Um exemplo seria um array mantendo as condições, através do qual você vai interagir com um tempo, e de impressão / break conforme necessário.

Existem muitas implementações, dependendo de suas necessidades para a criação de um código de exemplo seria inútil.

Como regra geral, quando seus olhares de código muito complicado, é uma porcaria :). Tente repensar isso. A seguir práticas de codificação na maioria das vezes torna o código por sua vez, fora muito mais estético e curto; e, obviamente, também de forma mais inteligente.

Código deverá reformular o problema em uma língua dada. Por isso eu afirmo que tanto trechos pode ser "melhor". Ele depende do problema que está sendo modelado. Embora o meu palpite é que nenhuma das soluções será paralelo o problema real. Se você colocar termos reais, em vez de condition1,2,3 que poderia mudar completamente o "melhor" de código.
Eu suspeito que há uma maneira melhor (3d) para escrever que todos juntos.

if( condition1 && condition2 && condition3 )
    return true;

log(String.Format("{0} failed", !condition1 ? "condition1" : (!condition2 ? "condition2" : "condition3")));
return false;

Dessa forma, você não tem que ver muitas linhas de código apenas para registro. E se todas as suas condições forem verdadeiras, você não perder tempo avaliando-os em caso você tem que entrar.

Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top