Declaração de mudança com retornos - correção do código

91

Digamos que eu tenha um código em C com aproximadamente esta estrutura:

switch (something)
{
    case 0:
      return "blah";
      break;

    case 1:
    case 4:
      return "foo";
      break;

    case 2:
    case 3:
      return "bar";
      break;

    default:
      return "foobar";
      break;
}

Obviamente, os breaks não são necessários para o código funcionar corretamente, mas parecerá uma prática ruim se eu não os colocar lá para mim.

O que você acha? Posso removê-los? Ou você os guardaria para aumentar a "correção"?

houbysoft
fonte

Respostas:

137

Remova as breakdeclarações. Eles não são necessários e talvez alguns compiladores emitam avisos de "Código inacessível" .

kgiannakakis
fonte
2
O mesmo se aplica a outras instruções de salto de controle incondicional, como continueou goto- é idiomático usá-las no lugar de break.
caf
31

Eu tomaria uma abordagem totalmente diferente. NÃO RETORNE no meio do método / função. Em vez disso, basta colocar o valor de retorno em uma variável local e enviá-lo no final.

Pessoalmente, acho o seguinte mais legível:

String result = "";

switch (something) {
case 0:
  result = "blah";
  break;
case 1:
  result = "foo";
  break;
}

return result;
Eu não
fonte
24
A filosofia "One Exit" faz sentido em C, onde você precisa garantir que as coisas sejam limpas corretamente. Considerando que em C ++ você pode ser arrancado da função em qualquer ponto por uma exceção, a filosofia de saída única não é realmente útil em C ++.
Billy ONeal
29
Em teoria, essa é uma ótima ideia, mas freqüentemente requer blocos de controle extras que podem dificultar a legibilidade.
mikerobi
8
A principal razão pela qual faço as coisas dessa maneira é que em funções mais longas é muito fácil perder o vetor de saída ao digitalizar o código. No entanto, quando a saída está sempre no final, é mais fácil entender rapidamente o que está acontecendo.
NotMe
7
Bem, isso e os blocos de retorno no meio do código apenas me lembram do abuso GOTO.
NotMe
5
@Chris: Em funções mais longas, concordo totalmente - mas isso geralmente se refere a problemas maiores, então refatore. Em muitos casos, é uma função trivial que retorna em um switch e é muito claro o que deve acontecer, portanto, não desperdice o poder do cérebro rastreando uma variável extra.
Stephen
8

Pessoalmente, gostaria de remover as devoluções e manter os intervalos. Eu usaria a instrução switch para atribuir um valor a uma variável. Em seguida, retorne essa variável após a instrução switch.

Embora este seja um ponto discutível, sempre achei que um bom design e encapsulamento significam uma entrada e uma saída. É muito mais fácil garantir a lógica e você não perderá acidentalmente o código de limpeza com base na complexidade ciclomática de sua função.

Uma exceção: retornar antecipadamente está correto se um parâmetro inválido for detectado no início de uma função - antes que quaisquer recursos sejam adquiridos.

Amardeep AC9MF
fonte
Pode ser bom para este particular switch, mas não necessariamente melhor em geral. Bem, digamos que a instrução switch dentro de uma função preceda outras 500 linhas de código que são válidas apenas quando certos casos são true. Qual é o objetivo de executar todo aquele código extra para os casos que não precisam executá-lo; não é melhor apenas returndentro do switchpara aqueles cases?
Fr0zenFyr
6

Mantenha as pausas - é menos provável que você tenha problemas se / quando editar o código mais tarde, se as quebras já estiverem no lugar.

Dito isso, é considerado por muitos (inclusive eu) uma má prática retornar do meio de uma função. Idealmente, uma função deve ter um ponto de entrada e um ponto de saída.

Paul R
fonte
5

Remova eles. É idiomático retornar de caseinstruções e, caso contrário, é ruído de "código inalcançável".

Stephen
fonte
5

Eu iria removê-los. Em meu livro, um código morto como esse deve ser considerado um erro porque faz você ficar surpreso e se perguntar "Como eu executaria essa linha?"

Hank Gay
fonte
4

Eu normalmente escreveria o código sem eles. IMO, código morto tende a indicar desleixo e / ou falta de compreensão.

Claro, eu também consideraria algo como:

char const *rets[] = {"blah", "foo", "bar"};

return rets[something];

Editar: mesmo com a postagem editada, essa ideia geral pode funcionar bem:

char const *rets[] = { "blah", "foo", "bar", "bar", "foo"};

if ((unsigned)something < 5)
    return rets[something]
return "foobar";

Em algum ponto, especialmente se os valores de entrada forem esparsos (por exemplo, 1, 100, 1000 e 10000), você deseja uma matriz esparsa. Você pode implementar isso como uma árvore ou um mapa razoavelmente bem (embora, é claro, uma opção ainda funcione neste caso também).

Jerry Coffin
fonte
Editou a postagem para refletir por que essa solução não funcionaria bem.
houbysoft
@sua edição: Sim, mas levaria mais memória, etc. IMHO, a mudança é a maneira mais simples, que é o que eu quero em uma função tão pequena (ela faz apenas a mudança).
houbysoft
3
@houbysoft: observe com atenção antes de concluir que isso exigirá memória extra. Com um compilador típico, usar "foo" duas vezes (por exemplo) usará dois ponteiros para um único bloco de dados, não replicará os dados. Você também pode esperar um código mais curto. A menos que você tenha muitos valores repetidos, geralmente economizará memória.
Jerry Coffin
verdade. No entanto, continuarei com a minha solução porque a lista de valores é muito longa e uma opção parece melhor.
houbysoft
1
De fato, a resposta mais elegante e eficiente para um problema como esse, quando os valores de switch são contíguos e o tipo de dados homogêneo, é usar um array para evitar um switch completamente.
Dwayne Robinson
2

Eu diria para removê-los e definir um padrão: branch.

Bella
fonte
Se não houver um padrão razoável, você não deve definir um padrão.
Billy ONeal
tem um, e eu defini um, só não coloquei na pergunta, editei agora.
houbysoft
2

Não seria melhor ter um array com

arr[0] = "blah"
arr[1] = "foo"
arr[2] = "bar"

e fazer return arr[something];?

Se for sobre a prática em geral, você deve manter as breakinstruções no switch. No caso de você não precisar de returndeclarações no futuro, diminui a chance de passar para a próxima case.

Vivin Paliath
fonte
Editou a postagem para refletir por que essa solução não funcionaria bem.
houbysoft
2

Para "correção", blocos de entrada única e saída única são uma boa ideia. Pelo menos eles eram quando eu fiz minha graduação em ciência da computação. Então, eu provavelmente declararia uma variável, atribuiria a ela no switch e retornaria uma vez no final da função

Steve Sheldon
fonte
2

O que você acha? Posso removê-los? Ou você os guardaria para aumentar a "correção"?

Não há problema em removê-los. Usar return é exatamente o cenário onde breaknão deve ser usado.

OscarRyz
fonte
1

Interessante. O consenso da maioria dessas respostas parece ser que a breakafirmação redundante é uma desordem desnecessária. Por outro lado, eu li a breakdeclaração em um switch como o 'encerramento' de um caso. caseblocos que não terminam em umbreak tendem a saltar sobre mim como uma queda potencial através de insetos.

Eu sei que não é assim quando há um em returnvez de um break, mas é assim que meus olhos 'lêem' os blocos de caso em um switch, então eu pessoalmente prefiro que cada um caseseja pareado com um break. Mas muitos compiladores reclamam que o breakdepois returné supérfluo / inacessível, e aparentemente eu pareço estar em minoria de qualquer maneira.

Portanto, livre-se do breakseguintereturn .

NB: tudo isso ignora se violar a regra de entrada / saída única é uma boa ideia ou não. Quanto a isso, tenho uma opinião que, infelizmente, muda dependendo das circunstâncias ...

Michael Burr
fonte
0

Eu digo para removê-los. Se o seu código é tão ilegível que você precisa colocar as quebras lá 'para estar no lado seguro', você deve reconsiderar seu estilo de codificação :)

Além disso, sempre preferi não misturar quebras e retornos na instrução switch, mas sim ficar com um deles.

Thomas Kjørnes
fonte
0

Eu pessoalmente tendo a perder o breaks. Possivelmente, uma fonte desse hábito são os procedimentos da janela de programação para aplicativos do Windows:

LRESULT WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
{
    switch (uMsg)
    {
        case WM_SIZE:
            return sizeHandler (...);
        case WM_DESTROY:
            return destroyHandler (...);
        ...
    }

    return DefWindowProc(hwnd, uMsg, wParam, lParam);
}

Pessoalmente, acho essa abordagem muito mais simples, sucinta e flexível do que declarar uma variável de retorno definida por cada manipulador e retorná-la no final. Dada essa abordagem, os breaks são redundantes e, portanto, devem ser eliminados - eles não têm nenhum propósito útil (sintaticamente ou IMO visualmente) e apenas incham o código.

Mac
fonte
0

Eu acho que o * intervalo * s têm um propósito. É para manter viva a 'ideologia' da programação. Se quisermos apenas 'programar' nosso código sem coerência lógica, talvez ele seja legível para você agora, mas tente amanhã. Tente explicar isso ao seu chefe. Tente executá-lo no Windows 3030.

Bleah, a ideia é muito simples:


Switch ( Algorithm )
{

 case 1:
 {
   Call_911;
   Jump;
 }**break**;
 case 2:
 {
   Call Samantha_28;
   Forget;
 }**break**;
 case 3:
 {
   Call it_a_day;
 }**break**;

Return thinkAboutIt?1:return 0;

void Samantha_28(int oBed)
{
   LONG way_from_right;
   SHORT Forget_is_my_job;
   LONG JMP_is_for_assembly;
   LONG assembly_I_work_for_cops;

   BOOL allOfTheAbove;

   int Elligence_says_anyways_thinkAboutIt_**break**_if_we_code_like_this_we_d_be_monkeys;

}
// Sometimes Programming is supposed to convey the meaning and the essence of the task at hand. It is // there to serve a purpose and to keep it alive. While you are not looking, your program is doing  // its thing. Do you trust it?
// This is how you can...
// ----------
// **Break**; Please, take a **Break**;

/ * Só uma pequena questão. Quanto café você bebeu durante a leitura do texto acima? TI quebra o sistema às vezes * /

Cruentos Solum
fonte
0

Saia do código em um ponto. Isso fornece melhor legibilidade para o código. Adicionar instruções de retorno (saídas múltiplas) entre elas tornará a depuração difícil.

antish
fonte
0

Se você tiver o tipo de código "lookup", poderá empacotar a cláusula switch-case em um método por si só.

Eu tenho alguns desses em um sistema de "hobby" que estou desenvolvendo para me divertir:

private int basePerCapitaIncomeRaw(int tl) {
    switch (tl) {
        case 0:     return 7500;
        case 1:     return 7800;
        case 2:     return 8100;
        case 3:     return 8400;
        case 4:     return 9600;
        case 5:     return 13000;
        case 6:     return 19000;
        case 7:     return 25000;
        case 8:     return 31000;
        case 9:     return 43000;
        case 10:    return 67000;
        case 11:    return 97000;
        default:    return 130000;
    }
}

(Sim. Esse é o espaço GURPS ...)

Concordo com os outros que você deve, na maioria dos casos, evitar mais de um retorno em um método e reconheço que este poderia ter sido melhor implementado como um array ou outra coisa. Acabei de descobrir que switch-case-return é uma combinação muito fácil para uma tabela de pesquisa com uma correlação de 1-1 entre entrada e saída, como a coisa acima (jogos de RPG estão cheios deles, tenho certeza de que existem em outros "negócios" também): D

Por outro lado, se a cláusula case for mais complexa, ou algo acontecer após a instrução switch, eu não recomendaria usar return nela, mas sim definir uma variável na switch, terminar com uma pausa e retornar o valor da variável no final.

(Por outro lado ... você sempre pode refatorar um switch em seu próprio método ... Duvido que tenha um efeito no desempenho, e não me surpreenderia se compiladores modernos pudessem reconhecê-lo como algo que poderia ser embutido ...)

Erk
fonte