Les variables de drapeau sont-elles un mal absolu? [fermé]

47

Les variables de drapeau sont-elles mauvaises? Les types de variables suivants sont-ils profondément immoraux et est-il mauvais de les utiliser?

"Les variables booléennes ou entières que vous attribuez une valeur à certains endroits puis en dessous vous permettent ensuite de faire quelque chose ou non, comme, par exemple, en utilisant newItem = trueensuite quelques lignes en dessous if (newItem ) then"


Je me souviens d’avoir fait deux projets où j’ai totalement négligé d’utiliser des drapeaux pour finir avec une meilleure architecture / code; Cependant, c'est une pratique courante dans d'autres projets pour lesquels je travaille et, lorsque le code croît et que des indicateurs sont ajoutés, le code-spaghetti IMHO se développe également.

Diriez-vous qu'il existe des cas où l'utilisation de drapeaux est une bonne pratique ou même nécessaire ?, ou seriez-vous d'accord pour dire que l'utilisation de drapeaux dans le code est ... un drapeau rouge et doit être évitée / refactorisée; moi, je me débrouille simplement avec des fonctions / méthodes qui vérifient les états en temps réel.

ducofgaming
la source
9
Il semble que MainMa et moi avons une définition différente de «drapeau». Je pensais au pré-processeur #ifdefs. De qui parliez-vous?
Karl Bielefeldt
C'est une très bonne question. Je me suis souvent posé cette question et je me suis retrouvé en train de dire: «eh bien, utilisons simplement un drapeau» un peu trop.
Paul Richter
Les booléens sont des drapeaux. (Il en va de même pour les nombres entiers, etc.)
Thomas Eding
7
@KarlBielefeldt Je pense que OP fait référence à des variables booléennes ou entières que vous attribuez une valeur à certains endroits, puis que vous vérifiez si vous souhaitez faire quelque chose ou non, comme, par exemple, en utilisant newItem = truequelques lignes ciif (newItem ) then
Tulains Córdova
1
Pensez également à l' introduction de la refactorisation de variable explicative dans ce contexte. Tant que la méthode reste courte et comporte un faible nombre de chemins, je considère cela comme une utilisation valable.
Daniel B

Réponses:

41

Le problème que j'ai constaté lors de la gestion de code utilisant des indicateurs est que le nombre d'états augmente rapidement et qu'il y a presque toujours des états non gérés. Un exemple de ma propre expérience: je travaillais sur un code qui avait ces trois drapeaux

bool capturing, processing, sending;

Ces trois pays ont créé huit États (en fait, il y avait également deux autres drapeaux). Toutes les combinaisons de valeurs possibles n'étaient pas couvertes par le code et les utilisateurs voyaient des bogues:

if(capturing && sending){ // we must be processing as well
...
}

Il s'est avéré qu'il y avait des situations où l'hypothèse dans la déclaration if ci-dessus était fausse.

Les drapeaux ont tendance à s'aggraver avec le temps et cachent l'état réel d'une classe. C'est pourquoi ils devraient être évités.

Ben
la source
3
+1, "devrait être évité". J'ajouterais quelque chose à propos de "mais des drapeaux sont nécessaires dans certaines situations" (certains diraient "un mal nécessaire")
Trevor Boyd Smith
2
@TrevorBoydSmith Selon mon expérience, ce n'est pas le cas, il vous suffit d'un peu plus que la puissance cérébrale moyenne que vous utiliseriez pour un drapeau
dukeofgaming
Dans votre exemple, cela aurait dû être une seule énumération représentant l'état, et non 3 booléens.
user949300
Vous pouvez arriver à un problème similaire à ce que je suis confronté en ce moment. En plus de couvrir tous les états possibles, deux applications peuvent partager le même drapeau (par exemple, le téléchargement de données client). Dans ce cas, un seul téléchargeur utilisera le drapeau, le déclenchera et la chance de trouver le problème à l'avenir.
Alan
38

Voici un exemple où les drapeaux sont utiles.

J'ai un morceau de code qui génère des mots de passe (en utilisant un générateur de nombres pseudo-aléatoires cryptographiquement sécurisé). L'appelant de la méthode choisit si le mot de passe doit contenir des lettres majuscules, des lettres minuscules, des chiffres, des symboles de base, des symboles étendus, des symboles grecs, des caractères cyrilliques et unicode.

Avec les drapeaux, appeler cette méthode est simple:

var password = this.PasswordGenerator.Generate(
    CharacterSet.Digits | CharacterSet.LowercaseLetters | CharacterSet.UppercaseLetters);

et il peut même être simplifié de:

var password = this.PasswordGenerator.Generate(CharacterSet.LettersAndDigits);

Sans drapeaux, quelle serait la signature de la méthode?

public byte[] Generate(
    bool uppercaseLetters, bool lowercaseLetters, bool digits, bool basicSymbols,
    bool extendedSymbols, bool greekLetters, bool cyrillicLetters, bool unicode);

appelé comme ça:

// Very readable, isn't it?
// Tell me just by looking at this code what symbols do I want to be included?
var password = this.PasswordGenerator.Generate(
    true, true, true, false, false, false, false, false);

Comme indiqué dans les commentaires, une autre approche consisterait à utiliser une collection:

var password = this.PasswordGenerator.Generate(
    new []
    {
        CharacterSet.Digits,
        CharacterSet.LowercaseLetters,
        CharacterSet.UppercaseLetters,
    });

Ceci est beaucoup plus lisible par rapport à trueet false, mais a toujours deux inconvénients:

L'inconvénient majeur est que pour permettre des valeurs combinées, CharacterSet.LettersAndDigitsvous écrivez quelque chose comme ça dans la Generate()méthode:

if (set.Contains(CharacterSet.LowercaseLetters) ||
    set.Contains(CharacterSet.Letters) ||
    set.Contains(CharacterSet.LettersAndDigits) ||
    set.Contains(CharacterSet.Default) ||
    set.Contains(CharacterSet.All))
{
    // The password should contain lowercase letters.
}

éventuellement réécrit comme ceci:

var lowercaseGroups = new []
{
    CharacterSet.LowercaseLetters,
    CharacterSet.Letters,
    CharacterSet.LettersAndDigits,
    CharacterSet.Default,
    CharacterSet.All,
};

if (lowercaseGroups.Any(s => set.Contains(s)))
{
    // The password should contain lowercase letters.
}

Comparez cela avec ce que vous avez en utilisant des drapeaux:

if (set & CharacterSet.LowercaseLetters == CharacterSet.LowercaseLetters)
{
    // The password should contain lowercase letters.
}

Le deuxième inconvénient très mineur est qu'il n'est pas clair comment la méthode se comporterait si elle était appelée ainsi:

var password = this.PasswordGenerator.Generate(
    new []
    {
        CharacterSet.Digits,
        CharacterSet.LettersAndDigits, // So digits are requested two times.
    });
Arseni Mourzenko
la source
10
Je crois que OP fait référence à des variables booléennes ou entières auxquelles vous attribuez une valeur à certains endroits, puis vous vérifiez si vous voulez faire quelque chose ou non, comme, par exemple, en utilisant newItem = truequelques lignes ciif (newItem ) then
Tulains Córdova
1
@MainMa Apparemment, il y a une 3ème: La version avec 8 arguments booléens est ce à quoi je pensais quand je lis "drapeaux" ...
Izkata
4
Désolé, mais à mon humble avis, c’est le cas idéal pour le chaînage de méthodes ( en.wikipedia.org/wiki/Method_chaining ). De plus, vous pouvez utiliser un tableau de paramètres (doit être un tableau associatif ou une carte), où toute entrée de ce tableau de paramètres vous omettez utilise le comportement de valeur par défaut pour ce paramètre. En fin de compte, l'appel via la chaîne de méthodes ou les tableaux de paramètres peut être aussi succinct et expressif que les indicateurs de bits. De plus, tous les langages n'ont pas d'opérateurs de bits (je préfère les indicateurs binaires, mais j'utiliserais plutôt les méthodes que je viens de mentionner).
dukeofgaming
3
Ce n'est pas très pratique, n'est-ce pas? Je créerais une interface ala: String myNewPassword = makePassword (randomComposeSupplier (new RandomLowerCaseSupplier (), new RandomUpperCaseSupplier (), new RandomNumberSupplier)); avec String makePassword (Supplier <Character> charSupplier); et Supplier <Character> randomComposeSupplier (fournisseurs <Character> ... fournisseurs); Vous pouvez désormais réutiliser vos fournisseurs pour d'autres tâches, les composer comme bon vous semble et simplifier votre méthode generatePassword afin qu'elle utilise un état minimal.
Dibbeke
4
@Dibbeke Parlez d'un royaume de noms ...
Phil
15

L'odeur est un énorme bloc fonctionnel, pas les drapeaux. Si vous définissez le drapeau sur la ligne 5, vérifiez uniquement si le drapeau est sur la ligne 354, alors c'est mauvais. Si vous définissez le drapeau sur la ligne 8 et vérifiez le drapeau sur la ligne 10, c'est bien. En outre, un ou deux indicateurs par bloc de code suffisent, 300 indicateurs dans une fonction sont incorrects.

nbv4
la source
10

Habituellement, les drapeaux peuvent être complètement remplacés par une variante du modèle de stratégie, avec une implémentation de stratégie pour chaque valeur possible du drapeau. Cela facilite l’ajout de nouveaux comportements.

Dans les situations critiques de performances, le coût de l'indirection peut faire surface et faire de la déconstruction un indicateur clair nécessaire. Cela étant dit, j'ai du mal à me souvenir d'un cas dans lequel je devais le faire.

back2dos
la source
6

Non, les drapeaux ne sont ni mauvais ni un mal qui doit être refait à tout prix.

Considérons l' appel Java Pattern.compile (regex String, int flags) . Ceci est un masque de bits traditionnel et cela fonctionne. Coup d' œil sur les constantes en Java et où vous voyez un groupe de 2 n vous savez qu'il ya des drapeaux étant là.

Dans un monde refactorisé idéal, on utilisera plutôt un EnumSet où les constantes sont des valeurs dans un enum et comme le dit la documentation:

Les performances spatio-temporelles de cette classe doivent être suffisantes pour permettre son utilisation en tant qu’alternative de type haute qualité et de haute qualité aux "indicateurs de bits" classiques basés sur int.

Dans un monde parfait, cet appel à Pattern.compile devient Pattern.compile(String regex, EnumSet<PatternFlagEnum> flags).

Cela dit, ça reste des drapeaux. Il est beaucoup plus facile de travailler avec ce Pattern.compile("foo", Pattern.CASE_INSENSTIVE | Pattern.MULTILINE)que ce ne serait le cas Pattern.compile("foo", new PatternFlags().caseInsenstive().multiline())ou avec un autre style d’essai de faire ce que les drapeaux sont vraiment et pour lequel ils sont bons.

Les drapeaux sont souvent visibles lorsque vous travaillez avec des objets de niveau système. Lors de l’interfaçage avec quelque chose au niveau du système d’exploitation, il est probable que l’on ait un indicateur quelque part, que ce soit la valeur de retour d’un processus, les autorisations d’un fichier ou les indicateurs d’ouverture d’un socket. Tenter de réorganiser ces événements lors d’une chasse aux sorcières contre une odeur de code perçue aboutira probablement à un code pire que si l’on utilisait accepté et comprenait le drapeau.

Le problème survient lorsque des personnes utilisent à mauvais escient des drapeaux en les jetant ensemble pour créer un ensemble de flancs francs de toutes sortes de drapeaux sans rapport ou en essayant de les utiliser là où ils ne sont pas du tout.


la source
5

Je suppose que nous parlons de drapeaux dans les signatures de méthodes.

Utiliser un seul drapeau est déjà assez grave.

Cela ne signifiera rien pour vos collègues la première fois qu’ils le verront. Ils devront examiner le code source de la méthode afin d’établir ce qu’elle fait. Vous serez probablement dans la même position quelques mois plus tard, lorsque vous oublierez le sens de votre méthode.

Passer un indicateur à la méthode signifie normalement que votre méthode est responsable de plusieurs choses. Dans la méthode, vous effectuez probablement une vérification simple sur les lignes suivantes:

if (flag)
   DoFlagSet();
else
   DoFlagNotSet();

C'est une mauvaise séparation des préoccupations et vous pouvez normalement trouver un moyen de la contourner.

J'ai normalement deux méthodes distinctes:

public void DoFlagSet() 
{
}

public void DoFlagNotSet()
{
}

Cela aura plus de sens avec les noms de méthodes applicables au problème que vous résolvez.

Passer plusieurs drapeaux est deux fois plus mauvais. Si vous avez réellement besoin de passer plusieurs indicateurs, envisagez de les encapsuler dans une classe. Même dans ce cas, vous serez toujours confronté au même problème, car votre méthode est susceptible de faire plusieurs choses.

CodeART
la source
3

Les drapeaux et la plupart des variables de température dégagent une forte odeur. Très probablement, ils pourraient être refactorisés et remplacés par des méthodes de requête.

Modifié:

Les drapeaux et les variables temp lors de l’expression de l’état doivent être refactorisés en méthodes de requête. Les valeurs d'état (booléens, entiers et autres primatives) devraient presque toujours être masquées dans les détails de la mise en œuvre.

Les drapeaux utilisés pour le contrôle, l'acheminement et le déroulement général du programme peuvent également indiquer la possibilité de reformuler des sections des structures de contrôle en stratégies ou usines distinctes, ou de faire ce qui est approprié en fonction de la situation, qui continuent à utiliser les méthodes de requête.

JustinC
la source
2

Lorsque nous parlons de drapeaux, nous devrions savoir qu'ils vont être modifiés au cours de l'exécution du programme et qu'ils vont affecter le comportement du programme en fonction de leurs états. Tant que nous maîtriserons parfaitement ces deux choses, elles fonctionneront très bien.

Les drapeaux peuvent très bien si

  • Vous les avez définis dans une portée appropriée. Par approprié, je veux dire que le champ d'application ne doit contenir aucun code qui ne doit pas / ne devrait pas être modifié. Ou du moins le code est sécurisé (par exemple, il ne peut pas être appelé directement de l'extérieur)
  • S'il est nécessaire de gérer les indicateurs de l'extérieur et s'il y a de nombreux indicateurs, nous pouvons coder le gestionnaire d'indicateurs comme le seul moyen de modifier les indicateurs en toute sécurité. Ce gestionnaire d'indicateurs peut lui-même encapsuler des indicateurs et des méthodes pour les modifier. Il peut être ensuite singleton, puis partagé entre les classes ayant besoin d'accéder aux indicateurs.
  • Et enfin pour la maintenabilité, s'il y a trop de drapeaux:
    • Inutile de dire qu'ils doivent suivre un nom judicieux
    • Doit être documenté avec ce qui sont des valeurs valides (peut être avec des énumérations)
    • Devrait être documenté avec le code QUI MODIFIERA chacun d’eux, ainsi que la condition QUELLE condition entraînera l’affectation d’une valeur particulière au drapeau.
    • QUEL CODE LES CONSUMENT ET QUEL COMPORTEMENT entraînera pour une valeur particulière

S'il y a beaucoup de drapeaux, un bon travail de conception doit précéder car ils commencent à jouer un rôle clé dans le comportement du programme. Vous pouvez choisir des diagrammes d'état pour la modélisation. De tels diagrammes servent également de documentation et de guidage visuel lors de l'utilisation.

Tant que ces choses seront en place, je pense que cela ne mènera pas au désordre.

Mahesha999
la source
1

A partir de la question, j'ai supposé que le QA était une variable de drapeau (globale), et non des bits d'un paramètre de fonction.

Il y a des situations où vous n'avez pas beaucoup d'autres possibilités. Par exemple, sans système d'exploitation, vous devez évaluer les interruptions. Si une interruption survient très fréquemment et que vous n'avez pas le temps de faire une longue évaluation dans l'ISR, il est non seulement permis, mais parfois même la meilleure pratique consiste à définir uniquement des drapeaux globaux dans l'ISR (vous devriez passer le moins de temps possible ISR) et d’évaluer ces indicateurs dans votre boucle principale.

vsz
la source
0

Je ne pense pas que rien soit un mal absolu dans la programmation, jamais.

Il y a une autre situation où des drapeaux pourraient être en ordre, qui n'étaient pas encore mentionnés ici ...

Considérez l'utilisation de fermetures dans cet extrait de code Javascript:

exports.isPostDraft = function ( post, draftTag ) {
  var isDraft = false;
  if (post.tags)
    post.tags.forEach(function(tag){ 
      if (tag === draftTag) isDraft = true;
    });
  return isDraft;
}

La fonction interne, étant passée à "Array.forEach", ne peut pas simplement "renvoyer true".

Par conséquent, vous devez conserver l'état extérieur avec un drapeau.

firstdoit
la source