Instruction Switch avec retours - correction du code

91

Disons que j'ai du code en C avec à peu près cette structure:

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;
}

Maintenant, évidemment, les breaks ne sont pas nécessaires pour que le code fonctionne correctement, mais cela ressemble à une mauvaise pratique si je ne les mets pas là.

Qu'est-ce que tu penses? Est-ce bien de les supprimer? Ou les conserveriez-vous pour une "exactitude" accrue?

houbysoft
la source

Réponses:

137

Supprimez les breakdéclarations. Ils ne sont pas nécessaires et peut-être que certains compilateurs émettront des avertissements "Code inaccessible" .

kgiannakakis
la source
2
La même chose s'applique avec d'autres instructions de saut de contrôle inconditionnelles, comme continueou goto- il est idiomatique de les utiliser à la place de break.
caf
32

Je prendrais une approche complètement différente. Ne retournez pas au milieu de la méthode / fonction. Au lieu de cela, mettez simplement la valeur de retour dans une variable locale et envoyez-la à la fin.

Personnellement, je trouve ce qui suit plus lisible:

String result = "";

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

return result;
Pas moi
la source
24
La philosophie "One Exit" est logique en C, où vous devez vous assurer que les choses sont correctement nettoyées. Étant donné qu'en C ++, vous pouvez être retiré de la fonction à tout moment par une exception, la philosophie de sortie unique n'est pas vraiment utile en C ++.
Billy ONeal
29
En théorie, c'est une excellente idée, mais elle nécessite souvent des blocs de contrôle supplémentaires qui peuvent nuire à la lisibilité.
mikerobi
8
La principale raison pour laquelle je fais les choses de cette façon, c'est que dans les fonctions plus longues, il est très facile de rater le vecteur de sortie lors de la numérisation du code. Cependant, lorsque la sortie est toujours à la fin, il est plus facile de comprendre rapidement ce qui se passe.
NotMe
7
Eh bien, cela, et les blocs de retour au milieu du code me rappellent simplement les abus de GOTO.
NotMe
5
@Chris: Dans les fonctions plus longues, je suis tout à fait d'accord - mais cela parle généralement de problèmes plus importants, alors refactorisez-le. Dans de nombreux cas, c'est une fonction triviale qui revient sur un commutateur, et ce qui est censé se passer est très clair, alors ne gaspillez pas votre cerveau en suivant une variable supplémentaire.
Stephen
8

Personnellement, je supprimerais les retours et garderais les pauses. J'utiliserais l'instruction switch pour attribuer une valeur à une variable. Puis renvoyez cette variable après l'instruction switch.

Bien que ce soit un point discutable, j'ai toujours pensé qu'une bonne conception et une bonne encapsulation signifiaient une entrée et une sortie. Il est beaucoup plus facile de garantir la logique et vous ne manquez pas accidentellement le code de nettoyage basé sur la complexité cyclomatique de votre fonction.

Une exception: le retour anticipé est acceptable si un paramètre incorrect est détecté au début d'une fonction - avant que des ressources ne soient acquises.

Amardeep AC9MF
la source
Cela pourrait être bon pour cela switch, mais pas nécessairement le meilleur en général. Eh bien, disons que l'instruction switch à l'intérieur d'une fonction précède 500 autres lignes de code qui ne sont valides que dans certains cas true. Quel est l'intérêt d'exécuter tout ce code supplémentaire pour les cas qui n'ont pas besoin de l'exécuter; n'est-il pas préférable de juste returnà l'intérieur switchpour ces cases
Fr0zenFyr
6

Gardez les pauses - vous êtes moins susceptible de rencontrer des problèmes si / lorsque vous modifiez le code plus tard si les pauses sont déjà en place.

Cela dit, beaucoup (y compris moi) considèrent comme une mauvaise pratique de revenir du milieu d'une fonction. Idéalement, une fonction doit avoir un point d'entrée et un point de sortie.

Paul R
la source
5

Retirez-les. Il est idiomatique de revenir à partir d' caseinstructions, et c'est du bruit de «code inaccessible» autrement.

Stephen
la source
5

Je les supprimerais. Dans mon livre, un code mort comme celui-là devrait être considéré comme une erreur car il vous oblige à faire une double prise et à vous demander "Comment pourrais-je exécuter cette ligne?"

Hank Gay
la source
4

J'écrirais normalement le code sans eux. OMI, le code mort a tendance à indiquer une négligence et / ou un manque de compréhension.

Bien sûr, j'envisagerais également quelque chose comme:

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

return rets[something];

Edit: même avec le post édité, cette idée générale peut bien fonctionner:

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

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

À un moment donné, en particulier si les valeurs d'entrée sont clairsemées (par exemple, 1, 100, 1000 et 10000), vous voulez plutôt un tableau épars. Vous pouvez l'implémenter sous forme d'arbre ou de carte raisonnablement bien (bien que, bien sûr, un commutateur fonctionne toujours dans ce cas également).

Jerry Coffin
la source
Modification de l'article pour expliquer pourquoi cette solution ne fonctionnerait pas bien.
houbysoft
@your edit: Oui, mais cela prendrait plus de mémoire, etc. À mon humble avis, le commutateur est le moyen le plus simple, ce que je veux dans une si petite fonction (il ne fait que le commutateur).
houbysoft
3
@houbysoft: regardez attentivement avant de conclure que cela prendra de la mémoire supplémentaire. Avec un compilateur typique, utiliser "foo" deux fois (par exemple) utilisera deux pointeurs vers un seul bloc de données, et non pas répliquer les données. Vous pouvez également vous attendre à un code plus court. À moins que vous n'ayez beaucoup de valeurs répétées, cela économisera souvent de la mémoire globale.
Jerry Coffin
vrai. Je vais quand même m'en tenir à ma solution car la liste de valeurs est très longue et un commutateur semble plus agréable.
houbysoft
1
En effet, la réponse la plus élégante et la plus efficace à un problème comme celui-ci, lorsque les valeurs des commutateurs sont contiguës et le type de données homogène, est d'utiliser un tableau pour éviter complètement un commutateur.
Dwayne Robinson
2

Je dirais les supprimer et définir un défaut: branche.

Bella
la source
S'il n'y a pas de valeur par défaut raisonnable, vous ne devez pas définir de valeur par défaut.
Billy ONeal
il y en a un, et j'en ai défini un, je ne l'ai simplement pas mis dans la question, je l'ai édité maintenant.
houbysoft
2

Ne serait-il pas préférable d'avoir un tableau avec

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

et faire return arr[something];?

S'il s'agit de la pratique en général, vous devez conserver les breakinstructions dans le commutateur. Dans le cas où vous n'avez pas besoin de returndéclarations à l'avenir, cela réduit le risque de passer à la suivante case.

Vivin Paliath
la source
Modification de l'article pour expliquer pourquoi cette solution ne fonctionnerait pas bien.
houbysoft
2

Pour "l'exactitude", les blocs à entrée unique et à sortie unique sont une bonne idée. Du moins, ils l'étaient quand j'ai obtenu mon diplôme en informatique. Donc je déclarerais probablement une variable, lui assignerais dans le commutateur et retournerais une fois à la fin de la fonction

Steve Sheldon
la source
2

Qu'est-ce que tu penses? Est-ce bien de les supprimer? Ou les conserveriez-vous pour une "exactitude" accrue?

C'est bien de les supprimer. L'utilisation return est exactement le scénario où breakne doit pas être utilisé.

OscarRyz
la source
1

Intéressant. Le consensus de la plupart de ces réponses semble être que la breakdéclaration redondante est un fouillis inutile. D'un autre côté, j'ai lu la breakdéclaration dans un interrupteur comme la «clôture» d'une affaire. caseles blocs qui ne se terminent pas par un breakont tendance à me sauter dessus comme une chute potentielle à cause de bugs.

Je sais que ce n'est pas comme ça quand il y a un returnau lieu d'un break, mais c'est ainsi que mes yeux `` lisent '' les blocs de boîtier dans un commutateur, donc je préférerais personnellement que chacun casesoit associé à un break. Mais de nombreux statisticiens ne se plaignent du breakaprès returnêtre superflu / injoignable, et apparemment je semblent être en minorité de toute façon.

Alors, débarrassez-vous de ce qui breaksuit a return.

NB: tout cela ne tient pas compte du fait que la violation de la règle d'entrée / sortie unique est une bonne idée ou non. Pour ce qui est de cela, j'ai un avis qui change malheureusement selon les circonstances ...

Michael Burr
la source
0

Je dis de les supprimer. Si votre code est tellement illisible que vous devez y insérer des pauses `` pour être du bon côté '', vous devriez reconsidérer votre style de codage :)

De plus, j'ai toujours préféré ne pas mélanger les pauses et les retours dans l'instruction switch, mais plutôt m'en tenir à l'un d'entre eux.

Thomas Kjørnes
la source
0

Personnellement, j'ai tendance à perdre le par break. L'une des sources de cette habitude est peut-être la programmation des procédures de fenêtre pour les applications 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);
}

Personnellement, je trouve cette approche beaucoup plus simple, succincte et flexible que de déclarer une variable de retour définie par chaque gestionnaire, puis de la renvoyer à la fin. Compte tenu de cette approche, les breaks sont redondants et devraient donc disparaître - ils ne servent à rien (syntaxiquement ou visuellement IMO) et ne font que gonfler le code.

Mac
la source
0

Je pense que les * break * s sont là pour un but. Il s'agit de maintenir vivante «l'idéologie» de la programmation. Si nous devons simplement «programmer» notre code sans cohérence logique, il vous sera peut-être lisible maintenant, mais essayez demain. Essayez de l'expliquer à votre patron. Essayez de l'exécuter sur Windows 3030.

Bleah, l'idée est très simple:


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**;

/ * Juste une question mineure. Combien de café avez-vous bu en lisant ce qui précède? IT casse parfois le système * /

Cruentos Solum
la source
0

Code de sortie à un moment donné. Cela offre une meilleure lisibilité du code. L'ajout d'instructions de retour (sorties multiples) entre les deux rendra le débogage difficile.

antish
la source
0

Si vous avez le type de code "recherche", vous pouvez empaqueter la clause switch-case dans une méthode par elle-même.

J'en ai quelques-uns dans un système "hobby" que je développe pour le plaisir:

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;
    }
}

(Ouais. C'est l'espace GURPS ...)

Je suis d'accord avec d'autres pour dire que vous devriez dans la plupart des cas éviter plus d'un retour dans une méthode, et je reconnais que celle-ci aurait pu être mieux implémentée en tant que tableau ou autre. Je viens de trouver que switch-case-return est une correspondance assez facile avec une table de recherche avec une corrélation 1-1 entre l'entrée et la sortie, comme la chose ci-dessus (les jeux de rôle en sont pleins, je suis sûr qu'ils existent dans d'autres "entreprises" également): D

D'un autre côté, si la clause case est plus complexe ou que quelque chose se passe après l'instruction switch, je ne recommanderais pas d'utiliser return dedans, mais plutôt de définir une variable dans le commutateur, de la terminer par une pause et de retourner la valeur de la variable à la fin.

(D'un autre côté ... vous pouvez toujours refactoriser un commutateur dans sa propre méthode ... Je doute que cela ait un effet sur les performances, et cela ne me surprendrait pas si les compilateurs modernes pouvaient même le reconnaître comme quelque chose qui pourrait être incorporé ...)

Erk
la source