Éviter le vaudou `goto`?

J'ai une switchstructure qui a plusieurs cas à gérer. Le switchopère sur un enumqui pose le problème de la duplication de code via des valeurs combinées:

// All possible combinations of One - Eight.
public enum ExampleEnum {
    One,
    Two, TwoOne,
    Three, ThreeOne, ThreeTwo, ThreeOneTwo,
    Four, FourOne, FourTwo, FourThree, FourOneTwo, FourOneThree,
          FourTwoThree, FourOneTwoThree
    // ETC.
}

Actuellement, la switchstructure traite chaque valeur séparément:

// All possible combinations of One - Eight.
switch (enumValue) {
    case One: DrawOne; break;
    case Two: DrawTwo; break;
    case TwoOne:
        DrawOne;
        DrawTwo;
        break;
     case Three: DrawThree; break;
     ...
}

Vous avez l'idée là. J'ai actuellement cette ifstructure en pile pour gérer les combinaisons avec une seule ligne:

// All possible combinations of One - Eight.
if (One || TwoOne || ThreeOne || ThreeOneTwo)
    DrawOne;
if (Two || TwoOne || ThreeTwo || ThreeOneTwo)
    DrawTwo;
if (Three || ThreeOne || ThreeTwo || ThreeOneTwo)
    DrawThree;

Cela pose le problème des évaluations logiques incroyablement longues, qui déroutent à la lecture et sont difficiles à maintenir. Après avoir réexaminé cette réflexion, j'ai commencé à réfléchir à des alternatives et à l'idée d'une switchstructure avec des retombées entre les cas.

Je dois utiliser un gotodans ce cas car C#ne permet pas de tomber à travers. Cependant, cela évite les chaînes logiques incroyablement longues, même s'il saute dans la switchstructure, tout en permettant la duplication de code.

switch (enumVal) {
    case ThreeOneTwo: DrawThree; goto case TwoOne;
    case ThreeTwo: DrawThree; goto case Two;
    case ThreeOne: DrawThree; goto default;
    case TwoOne: DrawTwo; goto default;
    case Two: DrawTwo; break;
    default: DrawOne; break;
}

Ce n'est toujours pas une solution suffisamment propre et le gotomot clé que je voudrais éviter est associé à une honte . Je suis sûr qu'il doit y avoir un meilleur moyen de nettoyer cela.

Ma question

Existe-t-il une meilleure façon de gérer ce cas spécifique sans affecter la lisibilité et la maintenabilité?

Je trouve le code difficile à lire avec les gotodéclarations. Je recommanderais de structurer votre enumdifféremment. Par exemple, si votre enumétait un champ de bits où chaque bit représentait l'un des choix, cela pourrait ressembler à ceci:

[Flags]
public enum ExampleEnum {
    One = 0b0001,
    Two = 0b0010,
    Three = 0b0100
};

L'attribut Flags indique au compilateur que vous définissez des valeurs qui ne se chevauchent pas. Le code qui appelle ce code peut définir le bit approprié. Vous pouvez ensuite faire quelque chose comme ceci pour clarifier ce qui se passe:

if (myEnum.HasFlag(ExampleEnum.One))
{
    CallOne();
}
if (myEnum.HasFlag(ExampleEnum.Two))
{
    CallTwo();
}
if (myEnum.HasFlag(ExampleEnum.Three))
{
    CallThree();
}

Cela nécessite le code qui définit myEnumpour définir les champs de bits correctement et marqué avec l'attribut Flags. Mais vous pouvez le faire en modifiant les valeurs des enums de votre exemple pour:

[Flags]
public enum ExampleEnum {
    One = 0b0001,
    Two = 0b0010,
    Three = 0b0100,
    OneAndTwo = One | Two,
    OneAndThree = One | Three,
    TwoAndThree = Two | Three
};

Lorsque vous écrivez un nombre dans le formulaire 0bxxxx, vous le spécifiez en binaire. Vous pouvez donc voir que nous avons défini les bits 1, 2 ou 3 (bien, techniquement, 0, 1 ou 2, mais vous avez l’idée). Vous pouvez également nommer des combinaisons en utilisant un bit OU si les combinaisons peuvent être fréquemment définies ensemble.

OMI la racine du problème est que ce morceau de code ne devrait même pas exister.

Vous avez apparemment trois conditions indépendantes et trois actions indépendantes à prendre si ces conditions sont vraies. Alors, pourquoi tout cela est-il canalisé dans un code qui a besoin de trois drapeaux booléens pour lui dire quoi faire (que vous les masquiez ou non en une énumération) et ensuite une combinaison de trois choses indépendantes? Le principe de la responsabilité unique semble avoir une journée de congé ici.

Placez les appels sur les trois fonctions auxquelles vous appartenez (c’est-à-dire où vous découvrez la nécessité de faire les actions) et consignez le code de ces exemples dans la corbeille.

S'il y avait dix drapeaux et trois actions, étendriez-vous ce type de code pour traiter 1024 combinaisons différentes? J'espère que non! Si 1024 est trop, 8 est aussi trop, pour la même raison.

Ne jamais utiliser gotos est l’un des concepts de «l’informatique » qui «ment aux enfants» . C'est le bon conseil 99% du temps, et les moments où il n'en est pas une sont tellement rares et spécialisés que c'est bien mieux pour tout le monde s'il est simplement expliqué aux nouveaux codeurs comme "ne les utilisez pas".

Alors, quand devraient- ils être utilisés? Il existe quelques scénarios , mais celui que vous semblez toucher est le suivant: lorsque vous codez une machine à états . S'il n'y a pas meilleure expression mieux organisée et structurée de votre algorithme qu'une machine à états, son expression naturelle dans le code implique des branches non structurées, et il n'y a pas grand chose à faire pour ce qui ne fait pas la structure du machine à états elle-même plus obscure, plutôt que moins.

Les rédacteurs de compilateurs le savent bien, c'est pourquoi le code source de la plupart des compilateurs qui implémentent des analyseurs LALR ^* contient des gotos. Cependant, très peu de gens coderont réellement leurs propres analyseurs et analyseurs syntaxiques lexicaux.

^{* - IIRC, il est possible de mettre en œuvre des grammaires LALL de descente entièrement récursive sans recourir à des tables de saut ou à d’autres instructions de contrôle non structurées. Par conséquent, si vous êtes vraiment anti-goto, c’est une solution.}

Maintenant, la question suivante est: "Cet exemple est-il l'un de ces cas?"

Ce que je vois, c'est que vous avez trois états différents possibles en fonction du traitement de l'état actuel. Puisque l’un d’eux ("default") n’est qu’une ligne de code, techniquement, vous pouvez vous en débarrasser en pointant cette ligne de code à la fin des états auxquels elle s’applique. Cela vous mènerait à 2 états possibles.

L'un des autres ("Trois") n'est branché qu'à un endroit que je peux voir. Donc, vous pouvez vous en débarrasser de la même manière. Vous vous retrouveriez avec un code qui ressemble à ceci:

switch (exampleValue) {
    case OneAndTwo: i += 3 break;
    case OneAndThree: i += 4 break;
    case Two: i += 2 break;
    case TwoAndThree: i += 5 break;
    case Three: i += 3 break;
    default: i++ break;
}

Cependant, encore une fois, c’était un exemple de jouet que vous avez fourni. Dans les cas où "par défaut" contient une quantité de code non négligeable, "trois" passe de plusieurs états à une transition, ou (plus important encore ) une maintenance supplémentaire risque d'ajouter ou de compliquer les états , vous seriez honnêtement mieux. utiliser gotos (et peut-être même se débarrasser de la structure enumère qui cache la nature de la machine à états, à moins qu'il y ait une bonne raison pour qu'elle reste).

La meilleure réponse est d' utiliser le polymorphisme .

Une autre réponse, qui, IMO, rend les choses si plus claires et peut-être plus courtes :

if (One || OneAndTwo || OneAndThree)
  CallOne();
if (Two || OneAndTwo || TwoAndThree)
  CallTwo();
if (Three || OneAndThree || TwoAndThree)
  CallThree();

goto est probablement mon 58ème choix ici ...

Pourquoi pas ceci:

public enum ExampleEnum {
    One = 0, // Why not?
    OneAndTwo,
    OneAndThree,
    Two,
    TwoAndThree,
    Three
}
int[] COUNTS = { 1, 3, 4, 2, 5, 3 }; // Whatever

int ComputeExampleValue(int i, ExampleEnum exampleValue) {
    return i + COUNTS[(int)exampleValue];
}

OK, je suis d’accord, c’est du hack (je ne suis pas un développeur C #, excusez-moi donc pour le code), mais du point de vue de l’efficacité, c’est un must? L'utilisation d'énums en tant qu'index de tableau est valide C #.

Si vous ne pouvez pas ou ne voulez pas utiliser les indicateurs, utilisez une fonction récursive. En mode de publication 64 bits, le compilateur produira un code très similaire à votre gotodéclaration. Vous n'avez simplement pas à vous en occuper.

int ComputeExampleValue(int i, ExampleEnum exampleValue) {
    switch (exampleValue) {
        case One: return i + 1;
        case OneAndTwo: return ComputeExampleValue(i + 2, ExampleEnum.One);
        case OneAndThree: return ComputeExampleValue(i + 3, ExampleEnum.One);
        case Two: return i + 2;
        case TwoAndThree: return ComputeExampleValue(i + 2, ExampleEnum.Three);
        case Three: return i + 3;
   }
}

La solution acceptée est correcte et constitue une solution concrète à votre problème. Cependant, je voudrais poser une solution alternative, plus abstraite.

D'après mon expérience, l'utilisation d'énums pour définir le flux de la logique constitue une odeur de code, car elle est souvent le signe d'une conception de classe médiocre.

J'ai rencontré un exemple concret de ce qui se passe dans le code sur lequel j'ai travaillé l'année dernière. Le développeur d'origine avait créé une classe unique, chargée de la logique d'importation et d'exportation, et basculant entre les deux sur la base d'une énumération. À présent, le code était similaire et comportait un code dupliqué, mais il était suffisamment différent pour rendre le code beaucoup plus difficile à lire et pratiquement impossible à tester. J'ai fini par refactoriser cela en deux classes distinctes, ce qui les a simplifiées et m'a permis de repérer et d'éliminer un certain nombre de bogues non signalés.

Encore une fois, je dois dire que l'utilisation d'énums pour contrôler le flux de la logique est souvent un problème de conception. Dans le cas général, Enums devrait être principalement utilisé pour fournir des valeurs adaptées au type et sécurisées pour le consommateur, lorsque les valeurs possibles sont clairement définies. Ils sont mieux utilisés en tant que propriété (par exemple, en tant qu'ID de colonne dans une table) qu'en tant que mécanisme de contrôle logique.

Considérons le problème présenté dans la question. Je ne connais pas vraiment le contexte ici, ni ce que représente cette énumération. Est-ce que c'est dessiner des cartes? Dessiner des images? Tirer du sang? L'ordre est-il important? Je ne sais pas non plus à quel point la performance est importante. Si les performances ou la mémoire sont essentielles, cette solution ne sera probablement pas celle que vous souhaitez.

Dans tous les cas, considérons l'énumération:

// All possible combinations of One - Eight.
public enum ExampleEnum {
    One,
    Two,
    TwoOne,
    Three,
    ThreeOne,
    ThreeTwo,
    ThreeOneTwo
}

Nous avons ici un certain nombre de valeurs enum différentes représentant différents concepts d’entreprise.

Ce que nous pourrions utiliser à la place, ce sont des abstractions pour simplifier les choses.

Considérons l'interface suivante:

public interface IExample
{
  void Draw();
}

Nous pouvons alors implémenter cela en tant que classe abstraite:

public abstract class ExampleClassBase : IExample
{
  public abstract void Draw();
  // other common functionality defined here
}

Nous pouvons avoir une classe concrète pour représenter les dessins un, deux et trois (qui, par souci d'argument, ont une logique différente). Ceux-ci pourraient potentiellement utiliser la classe de base définie ci-dessus, mais je suppose que le concept DrawOne est différent du concept représenté par l'énumération:

public class DrawOne
{
  public void Draw()
  {
    // Drawing logic here
  }
}

public class DrawTwo
{
  public void Draw()
  {
    // Drawing two logic here
  }
}

public class DrawThree
{
  public void Draw()
  {
    // Drawing three logic here
  }
}

Et maintenant, nous avons trois classes distinctes qui peuvent être composées pour fournir la logique aux autres classes.

public class One : ExampleClassBase
{
  private DrawOne drawOne;

  public One(DrawOne drawOne)
  {
    this.drawOne = drawOne;
  }

  public void Draw()
  {
    this.drawOne.Draw();
  }
}

public class TwoOne : ExampleClassBase
{
  private DrawOne drawOne;
  private DrawTwo drawTwo;

  public One(DrawOne drawOne, DrawTwo drawTwo)
  {
    this.drawOne = drawOne;
    this.drawTwo = drawTwo;
  }

  public void Draw()
  {
    this.drawOne.Draw();
    this.drawTwo.Draw();
  }
}

// the other six classes here

Cette approche est beaucoup plus verbeuse. Mais cela a des avantages.

Considérez la classe suivante, qui contient un bogue:

public class ThreeTwoOne : ExampleClassBase
{
  private DrawOne drawOne;
  private DrawTwo drawTwo;
  private DrawThree drawThree;

  public One(DrawOne drawOne, DrawTwo drawTwo, DrawThree drawThree)
  {
    this.drawOne = drawOne;
    this.drawTwo = drawTwo;
    this.drawThree = drawThree;
  }

  public void Draw()
  {
    this.drawOne.Draw();
    this.drawTwo.Draw();
  }
}

À quel point est-il plus simple de repérer l'appel drawThree.Draw () manquant? Et si l'ordre est important, l'ordre des appels de tirage au sort est également très facile à voir et à suivre.

Inconvénients de cette approche:

• Chacune des huit options présentées nécessite un cours séparé
• Cela utilisera plus de mémoire
• Cela rendra votre code superficiellement plus grand
• Parfois, cette approche n’est pas possible, bien que certaines variations puissent être possibles.

Avantages de cette approche:

• Chacune de ces classes est complètement testable; parce que
• La complexité cyclomatique des méthodes de tirage est faible (et en théorie, je pourrais me moquer des classes DrawOne, DrawTwo ou DrawThree si nécessaire)
• Les méthodes de tirage au sort sont compréhensibles - un développeur n'a pas à se lier le cerveau en travaillant à déterminer ce que la méthode fait.
• Les bugs sont faciles à repérer et difficiles à écrire
• Les classes se composent en classes de plus haut niveau, ce qui signifie qu'il est facile de définir une classe ThreeThreeThree

Considérez cette approche (ou une approche similaire) chaque fois que vous ressentez le besoin de rédiger un code de contrôle logique complexe dans les instructions de cas. Future, vous serez heureux de l'avoir fait.

Si vous souhaitez utiliser un commutateur ici, votre code sera réellement plus rapide si vous traitez chaque cas séparément.

switch(exampleValue)
{
    case One:
        i++;
        break;
    case Two:
        i += 2;
        break;
    case OneAndTwo:
    case Three:
        i+=3;
        break;
    case OneAndThree:
        i+=4;
        break;
    case TwoAndThree:
        i+=5;
        break;
}

une seule opération arithmétique est effectuée dans chaque cas

également, comme d’autres l’ont déjà dit, si vous envisagez d’utiliser gotos, vous devriez probablement repenser votre algorithme (bien que je concède que le manque de cas de C # peut être une raison pour utiliser un goto). Voir le célèbre article d'Edgar Dijkstra "Aller à la déclaration considérée comme nuisible"

Pour votre exemple particulier, puisque tout ce que vous vouliez vraiment de l'énumération était un indicateur "faire / ne pas-ne" pour chacune des étapes, la solution qui réécrit vos trois ifdéclarations est préférable à un switchet il est bon que vous en fassiez la réponse acceptée. .

Mais si vous aviez une logique plus complexe qui ne pas fonctionner de manière proprement, je trouve encore le gotos dans la switchdéclaration confusion. Je préférerais voir quelque chose comme ça:

switch (enumVal) {
    case ThreeOneTwo: DrawThree; DrawTwo; DrawOne; break;
    case ThreeTwo:    DrawThree; DrawTwo; break;
    case ThreeOne:    DrawThree; DrawOne; break;
    case TwoOne:      DrawTwo; DrawOne; break;
    case Two:         DrawTwo; break;
    default:          DrawOne; break;
}

Ce n'est pas parfait, mais je pense que c'est mieux ainsi qu'avec le gotos. Si les séquences d'événements sont si longues et se dupliquent tellement qu'il n'a pas vraiment de sens d'épeler la séquence complète pour chaque cas, je préférerais un sous-programme plutôt que gotopour réduire la duplication de code.

Je ne suis pas sûr que quiconque ait vraiment une raison de haïr le mot-clé goto de nos jours. C'est définitivement archaïque et inutile dans 99% des cas d'utilisation, mais c'est une caractéristique du langage pour une raison.

La raison pour détester le gotomot clé est un code comme

if (someCondition) {
    goto label;
}

string message = "Hello World!";

label:
Console.WriteLine(message);

Oops! Cela ne va clairement pas au travail. La messagevariable n'est pas définie dans ce chemin de code. Donc C # ne passera pas ça. Mais cela pourrait être implicite. Considérer

object.setMessage("Hello World!");

label:
object.display();

Et supposons qu’il displayy ait ensuite la WriteLinedéclaration.

Ce type de bogue peut être difficile à trouver car gotoobscurcit le chemin du code.

Ceci est un exemple simplifié. Supposons qu'un exemple réel ne soit pas aussi évident. Il peut y avoir cinquante lignes de code entre label:et l’utilisation de message.

Le langage peut aider à résoudre ce problème en limitant l'utilisation qui gotopeut être faite, en descendant seulement des blocs. Mais le C # goton'est pas limité comme ça. Il peut sauter par-dessus le code. De plus, si vous voulez limiter goto, il est également bon de changer le nom. D'autres langues sont utilisées breakpour descendre des blocs, avec un nombre (de blocs à quitter) ou une étiquette (sur l'un des blocs).

Le concept de gotoest une instruction de bas niveau en langage machine. Mais la raison principale pour laquelle nous avons des langages de niveau supérieur est que nous sommes limités aux abstractions de niveau supérieur, par exemple la portée variable.

Cela étant dit, si vous utilisez le C # gotodans une switchinstruction pour passer d'un cas à l'autre, c'est raisonnablement inoffensif. Chaque cas est déjà un point d'entrée. Je pense toujours que le qualifier de gotostupide dans cette situation, car il confond cet usage inoffensif de gotoavec des formes plus dangereuses. Je préférerais de beaucoup qu'ils utilisent quelque chose comme continueça pour ça. Mais pour une raison quelconque, ils ont oublié de me demander avant d’écrire la langue.

Lorsque vous avez autant de choix (et même plus, comme vous dites), alors peut-être que ce n'est pas du code mais des données.

Créez un dictionnaire mappant des valeurs d'énumération en actions, exprimé sous forme de fonctions ou sous la forme d'un type d'énum plus simple représentant les actions. Ensuite, votre code peut être réduit à une simple recherche dans un dictionnaire, suivi de l’appel de la valeur de la fonction ou de l’activation des choix simplifiés.

Utilisez une boucle et la différence entre pause et continuer.

do {
  switch (exampleValue) {
    case OneAndTwo: i += 2; break;
    case OneAndThree: i += 3; break;
    case Two: i += 2; continue;
    case TwoAndThree: i += 2;
    // dropthrough
    case Three: i += 3; continue;
    default: break;
  }
  i++;
} while(0);