Melhor prática sobre se / retornar

51

Eu quero saber qual é a melhor maneira de retornar quando eu tenho if declaração.

Exemplo 1:

public bool MyFunction()
{
   // Get some string for this example
   string myString = GetString();

   if (myString == null)
   {
      return false;
   }
   else
   {
      myString = "Name " + myString;
      // Do something more here...
      return true;
   }
}

Exemplo 2:

public bool MyFunction()
{
   // Get some string for this example
   string myString = GetString();

   if (myString == null)
   {
      return false;
   }

   myString = "Name " + myString;
   // Do something more here...
   return true;
}

Como você pode ver nos dois exemplos, a função retornará true/false , mas é uma boa idéia colocar a declaração else como no primeiro exemplo ou é melhor não colocá-la?

    
por stan 19.07.2012 / 15:05
fonte

16 respostas

76

O exemplo 2 é conhecido como bloco de proteção. É mais adequado para retornar / lançar exceção antecipada se algo deu errado (parâmetro incorreto ou estado inválido). No fluxo lógico normal, é melhor usar o Exemplo 1

    
por 19.07.2012 / 15:20
fonte
42

Meu estilo pessoal é usar o único if para blocos de proteção e o if / else no código de processamento do método real.

Nesse caso, você está usando a condição myString == null como de guarda, por isso eu costumava usar o único padrão if .

Considere o código que é um pouco mais complicado:

Exemplo 1:

public bool MyFunction(myString: string){

    //guard block
    if (myString == null){
        return false;
    }
    else{
        //processing block
        myString = escapedString(myString);

        if (myString == "foo"){
            //some processing here
            return false;
        }
        else{
            myString = "Name " + myString;
            //other stuff
            return true;
        }
    }
}

Exemplo 2:

public bool MyFunction(myString: string){

    //guard block
    if (myString == null){
        return false;
    }

    //processing block
    myString = escapedString(myString);

    if (myString == "foo"){
        //some processing here
        return false;
    }
    else{
        myString = "Name " + myString;
        //other stuff
        return true;
    }
}

No Exemplo 1, o guarda e o restante do método estão no formato if / else . Compare isso com o Exemplo 2, em que o bloco de proteção está no único formulário if , enquanto o restante do método usa o formato if / else . Pessoalmente, acho o exemplo 2 mais fácil de entender, enquanto o exemplo 1 parece confuso e excessivamente recuado.

Observe que este é um exemplo artificial e que você pode usar as instruções else if para limpá-lo, mas pretendo mostrar a diferença entre os blocos de proteção e o código de processamento da função real.

Um compilador decente deve gerar a mesma saída para ambos de qualquer maneira. A única razão para usar um ou outro é a preferência pessoal ou estar em conformidade com o estilo do código existente.

    
por 13.04.2016 / 23:26
fonte
18

Pessoalmente, eu prefiro o segundo método. Eu sinto que é mais curto, tem menos recuo e é mais fácil de ler.

    
por 19.07.2012 / 15:23
fonte
15

Minha prática pessoal é a seguinte:

  • Eu não gosto de funções com vários pontos de saída, achei difícil manter e seguir, modificações de código, por vezes, quebram a lógica interna, porque é inerentemente um pouco desleixado. Quando é um cálculo complexo, eu crio um valor de retorno no início e o retorno no final. Isso me obriga a seguir cuidadosamente cada um dos caminhos if-else, switch, etc, definir o valor corretamente nos locais apropriados. Também gasto um pouco de tempo na decisão de definir um valor de retorno padrão ou deixá-lo não inicializado no início. Esse método também ajuda quando a lógica, o tipo de valor de retorno ou o significado muda.

Por exemplo:

public bool myFunction()
{
   // First parameter loading
   String myString = getString();

   // Location of "quick exits", see the second example
   // ...

   // declaration of external resources that MUST be released whatever happens
   // ...

   // the return variable (should think about giving it a default value or not) 
   // if you have no default value, declare it final! You will get compiler 
   // error when you try to set it multiple times or leave uninitialized!
   bool didSomething = false;

   try {
     if (myString != null)
     {
       myString = "Name " + myString;
       // Do something more here...

       didSomething = true;
     } else {
       // get other parameters and data
       if ( other conditions apply ) {
         // do something else
         didSomething = true;
       }
     }

     // Edit: previously forgot the most important advantage of this version
     // *** HOUSEKEEPING!!! ***

   } finally {

     // this is the common place to release all resources, reset all state variables

     // Yes, if you use try-finally, you will get here from any internal returns too.
     // As I said, it is only my taste that I like to have one, straightforward path 
     // leading here, and this works even if you don't use the try-finally version.

   }

   return didSomething;
}
  • A única exceção: "saída rápida" no início (ou, em casos raros, dentro do processo). Se a lógica de cálculo real não puder lidar com uma certa combinação de parâmetros de entrada e estados internos, ou se tiver uma solução fácil sem executar o algoritmo, não ajudará ter todo o código encapsulado em blocos (às vezes profundos). Esse é um "estado excepcional", que não faz parte da lógica principal, portanto, preciso sair do cálculo assim que for detectado. Neste caso não há mais ramificações, em condições normais a execução simplesmente continua. (É claro que "estado excepcional" é melhor expresso ao lançar uma exceção, mas às vezes é um exagero).

Por exemplo:

public bool myFunction()
{
   String myString = getString();

   if (null == myString)
   {
     // there is nothing to do if myString is null
     return false;
   } 

   myString = "Name " + myString;
   // Do something more here...

   // not using return value variable now, because the operation is straightforward.
   // if the operation is complex, use the variable as the previous example.

   return true;
}

A regra "uma saída" também ajuda quando o cálculo requer recursos externos que você deve liberar ou afirma que você precisa redefinir antes de sair da função; às vezes eles são adicionados mais tarde durante o desenvolvimento. Com várias saídas dentro do algoritmo, é muito mais difícil estender todos os ramos adequadamente. (E se exceções podem ocorrer, o release / reset deve ser colocado em um bloco final também, para evitar efeitos colaterais em casos excepcionais raros ...).

Seu caso parece cair na categoria "saída rápida antes do trabalho real", e eu escreveria como sua versão do exemplo 2.

    
por 20.07.2012 / 09:54
fonte
8

Eu prefiro usar bloqueios de guarda sempre que posso por dois motivos:

  1. Eles permitem uma saída rápida, dada alguma condição específica.
  2. Remova a necessidade de instruções complexas e desnecessárias se as instruções forem posteriores no código.

De um modo geral, prefiro ver métodos em que a funcionalidade principal do método é clara e mínima. Blocos de proteção ajudam a fazer isso acontecer visualmente.

    
por 20.07.2012 / 01:26
fonte
4

Eu gosto da abordagem "Fall Through":

public bool MyFunction()
{
   string myString = GetString();

   if (myString != null)
   {
     myString = "Name " + myString;
     return true;
    }
    return false;
}

A ação tem uma condição específica, qualquer outra coisa é apenas o retorno padrão "fall through".

    
por 19.07.2012 / 15:52
fonte
1

Se eu tiver uma única condição, não gastaria muito tempo ruminando sobre o estilo. Mas se eu tiver várias condições de guarda, prefiro style2

Faça um piquenique. Suponha que os testes sejam complexos e você realmente não queira amarrá-los em uma única condição se-ORed para evitar a complexidade:

//Style1
if (this1 != Right)
{ 
    return;
}
else if(this2 != right2)
{
    return;
}
else if(this3 != right2)
{
    return;
}
else
{
    //everything is right
    //do something
    return;
}

versus

//Style 2
if (this1 != Right)
{ 
   return;
}
if(this2 != right2)
{
    return;
}
if(this3 != right2)
{
    return;
}


//everything is right
//do something
return;

Aqui existem duas vantagens principais

  1. Você está separando o código em uma única função em dois blocos logicamente visuais: um bloco superior de validações (guarda condições) e um bloco inferior de código executável.

  2. Se você precisar adicionar / remover uma condição, reduz suas chances de estragar toda a escada if-elseif-else.

Outra pequena vantagem é que você tem menos um par de chaves para cuidar.

    
por 27.07.2015 / 00:05
fonte
0

Esta deve ser uma questão que soa melhor.

If condition
  do something
else
  do somethingelse

se expressa melhor do que

if condition
  do something
do somethingelse

para métodos pequenos, não será uma grande diferença, mas para métodos compostos maiores, pode ser mais difícil entender como ele não será adequadamente separado

    
por 19.07.2012 / 15:15
fonte
0

Acho o exemplo 1 irritante, porque a declaração de retorno ausente no final de uma função de retorno de valor aciona imediatamente a mensagem "Espere, há algo errado". Então, em casos como este, eu iria com o exemplo 2.

No entanto, geralmente há mais envolvido, dependendo do propósito da função, e. tratamento de erros, registro, etc. Portanto, estou com a resposta de Lorand Kedves sobre isso e geralmente tenho um ponto de saída no final, mesmo ao custo de uma variável de sinalização adicional. Isso facilita a manutenção e as extensões posteriores na maioria dos casos.

    
por 19.07.2012 / 15:47
fonte
0

Quando sua instrução if sempre retorna, não há razão para usar outra para o código restante na função. Isso adiciona linhas extras e recuo extra. Adicionar código desnecessário dificulta a leitura e, como todos sabem, O código de leitura é difícil .

    
por 19.07.2012 / 19:27
fonte
0

No seu exemplo, o else é obviamente desnecessário, MAS ...

Ao deslizar rapidamente através de linhas de código, os olhos normalmente olham para as chaves e os recuos para se ter uma ideia do fluxo de código; antes que eles realmente resolvam o código em si. Portanto, gravar o else e colocar chaves e indentação em torno do segundo bloco de código, na verdade, torna mais rápido para o leitor ver que essa é uma situação "A ou B", na qual um bloco ou outro será executado. Ou seja, o leitor vê as chaves e a indentação ANTES de ver o return após o if inicial.

Na minha opinião, este é um daqueles casos raros em que adicionar algo redundante ao código realmente torna-o MAIS legível, não menos.

    
por 20.07.2012 / 09:32
fonte
0

Eu prefiro o exemplo 2 porque é imediatamente óbvio que a função retorna alguma coisa . Mas mais do que isso, prefiro voltar de um lugar, assim:

public bool MyFunction()
{
    bool result = false;

    string myString = GetString();

    if (myString != nil) {
        myString = "Name " + myString;

        result = true;
    }

    return result;
}

Com esse estilo, posso:

  1. Veja imediatamente que estou retornando algo.

  2. Capture o resultado de cada chamada da função com apenas um ponto de interrupção.

por 20.07.2012 / 09:55
fonte
0

Meu estilo pessoal tende a ir

function doQuery(string) {
    if (!query(string)) {
        query("ABORT");
        return false;
    } // else
    if(!anotherquery(string) {
        query("ABORT");
        return false;
    } // else
    return true;
}

Usando as instruções else comentado para indicar o fluxo do programa e mantê-lo legível, mas evitando recuos maciços que podem ser facilmente alcançados na tela se muitas etapas estiverem envolvidas.

    
por 20.07.2012 / 10:13
fonte
0

Eu escreveria

public bool SetUserId(int id)
{
   // Get user name from id
   string userName = GetNameById(id);

   if (userName != null)
   {
       // update local ID and userName
       _id = id;
       _userNameLabelText = "Name: " + userName;
   }

   return userName != null;
}

Eu prefiro este estilo porque é mais óbvio que eu retornaria userName != null .

    
por 20.07.2012 / 11:03
fonte
-1

Minha regra básica é fazer um if-return sem mais (principalmente para erros) ou fazer uma cadeia completa if-else-if sobre todas as possibilidades.

Desta forma evito tentar ser muito extravagante com minha ramificação, já que sempre que termino fazendo isso codifico a minha aplicação em meus ifs, tornando-os mais difíceis de manter (por exemplo, se um enum OK ou ERR e eu escrevo todos os meus ifs aproveitando o fato de que! OK < - > ERR torna-se uma dor na bunda para adicionar uma terceira opção para o enum da próxima vez.)

No seu caso, por exemplo, eu usaria um plain if, já que "return if null" é certeza de ser um padrão comum e não é provável que você precise se preocupar com uma terceira possibilidade (além de null / não nulo) no futuro.

No entanto, se o teste fosse algo que fizesse uma inspeção mais aprofundada sobre os dados, eu iria errar em direção a um if-else completo.

    
por 20.07.2012 / 07:53
fonte
-1

Eu acho que há muitas armadilhas para o Exemplo 2, que no caminho pode levar a um código não intencional. Primeiro, o foco aqui é baseado na lógica em torno da variável 'myString'. Portanto, para ser explícito, todos os testes de condições devem acontecer em um bloco de código que considera a lógica conhecida e padrão / desconhecido .

E se mais tarde o código fosse introduzido involuntariamente no exemplo 2, que alterou significativamente a saída:

   if (myString == null)
   {
      return false;
   }

   //add some v1 update code here...
   myString = "And the winner is: ";
   //add some v2 update code here...
   //buried in v2 updates the following line was added
   myString = null;
   //add some v3 update code here...
   //Well technically this should not be hit because myString = null
   //but we already passed that logic
   myString = "Name " + myString;
   // Do something more here...
   return true;

Acho que com o bloco else imediatamente após a verificação de um nulo, os programadores que adicionaram os aprimoramentos às versões futuras adicionaram toda a lógica, porque agora temos uma sequência de lógica que não foi intencional para a regra original (retornando se o valor for nulo).

Acredito muito nisso em algumas diretrizes do C # no Codeplex (link para isso aqui: link ) que afirmam seguinte:

"Adicione um comentário descritivo se o bloco padrão (else) estiver vazio. Além disso, se esse bloco não for atingido, lance uma InvalidOperationException para detectar alterações futuras que podem cair nos casos existentes. Isso garante um código melhor, porque todos os caminhos que o código pode percorrer foram pensados. "

Eu acho que é uma boa prática de programação usar blocos lógicos como esse para sempre ter um bloco padrão adicionado (if-else, case: default) para explicar explicitamente todos os caminhos de código e não deixar código aberto para conseqüências lógicas não intencionais.

    
por 20.07.2012 / 16:39
fonte