Prática recomendada em caso de retorno

Quero saber o que é considerado a melhor maneira de retornar quando tenho uma ifdeclaraçã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/falsemas é uma boa ideia colocar uma elsedeclaração como no primeiro exemplo ou é melhor não colocá-la?

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

Meu estilo pessoal é usar o single ifpara blocos de proteção e o if/ elseno código de processamento do método real.

Nesse caso, você está usando a myString == nullcondição de guarda, então eu tenderia a usar o ifpadrão único .

Considere o código 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, a proteção e o restante do método estão no formato if/ else. Compare isso ao Exemplo 2, onde o bloco de proteção está no ifformato único , enquanto o restante do método usa o if/ elseformulário. Pessoalmente, acho o exemplo 2 mais fácil de entender, enquanto o exemplo 1 parece confuso e com recuo excessivo.

Observe que este é um exemplo artificial e que você pode usar else ifinstruções 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 os dois de qualquer maneira. O único motivo para usar um ou outro é a preferência pessoal ou a conformidade com o estilo do código existente.

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

Minha prática pessoal é a seguinte:

• Não gosto de funções com vários pontos de saída, achei difícil manter e seguir; as modificações de código às vezes quebram a lógica interna porque é inerentemente um pouco desleixada. Quando se trata de um cálculo complexo, crio um valor de retorno no início e o retorno no final. Isso me obriga a seguir cuidadosamente cada caminho de if-else, switch, etc., definir o valor corretamente nos locais adequados. Também passo um pouco de tempo decidindo se devo 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 ou o tipo ou significado do valor de retorno é alterado.

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 pode lidar com uma certa combinação de parâmetros de entrada e estados internos, ou tem uma solução fácil sem executar o algoritmo, não ajuda ter todo o código encapsulado em blocos (às vezes profundos). Este é um "estado excepcional", que não faz parte da lógica principal, portanto, devo sair do cálculo assim que detectar. Nesse caso, não há mais ramo, 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ê precisa liberar ou afirma que é necessário redefinir antes de sair da função; às vezes, são adicionados posteriormente durante o desenvolvimento. Com várias saídas dentro do algoritmo, é muito mais difícil estender todas as ramificações corretamente. (E, se houver exceções, a liberação / redefinição também deve ser bloqueada para evitar efeitos colaterais em casos excepcionais raros ...).

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

Prefiro empregar blocos de guarda sempre que possível por dois motivos:

1. Eles permitem uma saída rápida, devido a alguma condição específica.
2. Remove a necessidade de instruções if complexas e desnecessárias posteriormente 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.

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 "fall through" padrão.

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

Picuture isso. Suponha que os testes sejam complexos e você realmente não deseja vinculá-los a uma única condição if-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 visualmente logais: um bloco superior de validações (condições de guarda) e um bloco inferior de código executável.

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

Outra pequena vantagem é que você tem um conjunto a menos de aparelhos para cuidar.

Isso deve ser uma questão de que soa melhor.

If condition
  do something
else
  do somethingelse

se expressa melhor então

if condition
  do something
do somethingelse

para métodos pequenos, não haverá muita diferença, mas para métodos compostos maiores, pode ser mais difícil de entender, pois não será adequadamente separado.

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

No entanto, geralmente há mais envolvimento, dependendo do objetivo da função, por exemplo, tratamento de erros, registro etc. Então, eu estou com a resposta de Lorand Kedves sobre isso, e geralmente tenho um ponto de saída no final, mesmo com o custo de uma variável de flag adicional. Facilita a manutenção e as extensões posteriores na maioria dos casos.

Quando sua ifinstrução sempre retorna, não há razão para usar uma outra coisa para o código restante na função. Fazer isso adiciona linhas extras e recuo extra. A adição de código desnecessário torna mais difícil a leitura e, como todos sabem, o código de leitura é difícil .

No seu exemplo, o elseobviamente é desnecessário, MAS ...

Ao percorrer rapidamente as linhas de código, os olhos normalmente olham para os aparelhos e os recuos para ter uma idéia do fluxo de código; antes que eles realmente se instalem no próprio código. Portanto, escrever elsee colocar chaves e recuo em torno do segundo bloco de código 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 o recuo antes de ver o returndepois da inicial if.

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

Eu prefiro o exemplo 2 porque é imediatamente óbvio que a função retorna algo . Mas mais do que isso, eu preferiria retornar de um lugar, assim:

public bool MyFunction()
{
    bool result = false;

    string myString = GetString();

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

        result = true;
    }

    return result;
}

Com este estilo eu 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.

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 elseinstruções comentadas para indicar o fluxo do programa e mantê-lo legível, mas evitando recuos massivos que podem chegar facilmente na tela se houver várias etapas envolvidas.

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 esse estilo porque é mais óbvio que eu voltaria userName != null.

Minha regra de ouro é fazer um if-return sem outro (principalmente por erros) ou uma cadeia if-else-if completa sobre todas as possibilidades.

Dessa forma, evito tentar ser extravagante demais com minha ramificação, pois sempre que acabo fazendo isso codifico a lógica do aplicativo nos meus ifs, dificultando sua manutenção (por exemplo, se um enum está OK ou ERR e escrevo todos os meus ifs tirando proveito do fato de que! OK <-> ERR, torna-se uma chatice adicionar uma terceira opção ao enum da próxima vez.)

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

No entanto, se o teste fosse algo que fizesse uma inspeção mais aprofundada dos dados, eu iria errar no sentido de um if-else completo.

Penso que existem muitas armadilhas no Exemplo 2, que no futuro poderiam levar a códigos não intencionais. O primeiro do foco aqui é baseado na lógica em torno da variável 'myString'. Portanto, para ser explícito, todos os testes de condições devem ocorrer em um bloco de código que contabilize lógica conhecida e padrão / desconhecida .

E se, mais tarde, o código for introduzido acidentalmente 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;

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

Eu acredito muito nisso em algumas das diretrizes C # no Codeplex (link para isso aqui: http://csharpguidelines.codeplex.com/ ) que afirmam o 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 possam ocorrer nos casos existentes. Isso garante um código melhor, porque todos os caminhos que o código pode percorrer foram considerados ".

Eu acho que é uma boa prática de programação ao usar blocos lógicos como este, sempre adicionar um bloco padrão (caso contrário, case: default) para explicar explicitamente todos os caminhos de código e não deixar o código aberto para consequências lógicas não intencionais.