Le variabili flag sono un male assoluto? [chiuso]

Il problema che ho riscontrato quando si gestiva il codice che fa uso di flag è che il numero di stati aumenta rapidamente e ci sono quasi sempre stati non gestiti. Un esempio della mia esperienza personale: stavo lavorando su un codice che aveva queste tre bandiere

bool capturing, processing, sending;

Questi tre hanno creato otto stati (in realtà c'erano anche altri due flag). Non tutte le possibili combinazioni di valori erano coperte dal codice e gli utenti vedevano bug:

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

Si è scoperto che c'erano situazioni in cui l'assunto nella dichiarazione if era falso.

Le bandiere tendono a espandersi nel tempo e nascondono lo stato attuale di una classe. Ecco perché dovrebbero essere evitati.

Ecco un esempio in cui i flag sono utili.

Ho un pezzo di codice che genera password (usando un generatore di numeri pseudocasuali crittograficamente sicuro). Il chiamante del metodo sceglie se la password deve contenere lettere maiuscole, minuscole, cifre, simboli di base, simboli estesi, simboli greci, caratteri cirillici e unicode.

Con i flag, chiamare questo metodo è semplice:

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

e può anche essere semplificato in:

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

Senza flag, quale sarebbe la firma del metodo?

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

chiamato in questo modo:

// 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);

Come notato nei commenti, un altro approccio sarebbe utilizzare una raccolta:

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

Questo è molto più leggibile rispetto al set di true e false , ma ha ancora due svantaggi:

Lo svantaggio principale è che, per consentire valori combinati, come CharacterSet.LettersAndDigits dovresti scrivere qualcosa del genere in Generate() metodo:

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

probabilmente riscritto in questo modo:

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

Confronta questo con quello che hai usando i flag:

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

Il secondo svantaggio è che non è chiaro come si comporterebbe il metodo se chiamato così:

var password = this.PasswordGenerator.Generate(
    new []
    {
        CharacterSet.Digits,
        CharacterSet.LettersAndDigits, // So digits are requested two times.
    });

Un enorme blocco funzione è l'odore, non le bandiere. Se imposti il flag sulla riga 5, controlla solo il flag sulla riga 354, quindi non va bene. Se si imposta il flag sulla linea 8 e si controlla la flag sulla riga 10, va bene. Inoltre, uno o due flag per blocco di codice vanno bene, 300 flag in una funzione sono negativi.

Di solito i flag possono essere completamente sostituiti da alcuni aspetti del modello di strategia, con un'implementazione della strategia per ogni possibile valore della bandiera. Ciò rende molto più semplice l'aggiunta di nuovi comportamenti.

In situazioni critiche per le prestazioni, il costo di potrebbe indirizzare e rendere necessaria la decostruzione in flag chiari. Detto questo, ho problemi a ricordare un singolo caso in cui dovevo farlo.

No, le bandiere non sono male o un male che deve essere cancellato a tutti i costi.

Considera Java Pattern.compile (String regex, int flags) call. Questa è una maschera di bit tradizionale e funziona. Dai un'occhiata alle costanti in java e ovunque vedi un gruppo di 2 ⁿ sai che ci sono bandiere lì.

In un mondo refactored ideale, si dovrebbe invece usare un EnumSet dove le costanti sono invece valori in un enum e come dice la documentazione:

The space and time performance of this class should be good enough to allow its use as a high-quality, typesafe alternative to traditional int-based "bit flags."

In un mondo perfetto, la chiamata a Pattern.compile diventa Pattern.compile(String regex, EnumSet<PatternFlagEnum> flags) .

Tutto ciò che ha detto, è ancora bandiere. È molto più facile lavorare con Pattern.compile("foo", Pattern.CASE_INSENSTIVE | Pattern.MULTILINE) di quanto vorrebbe avere Pattern.compile("foo", new PatternFlags().caseInsenstive().multiline()) o qualche altro stile di tentativo di fare ciò che i flag sono realmente e buoni.

Le bandiere si vedono spesso quando si lavora con cose a livello di sistema. Quando ci si interfaccia con qualcosa a livello di sistema operativo, è probabile che si abbia un flag da qualche parte, che si tratti del valore di ritorno di un processo, o delle autorizzazioni di un file, o dei flag per l'apertura di un socket. Cercare di rifattorizzare queste istanze in una caccia alle streghe contro un odore di codice percepito probabilmente finirà con un codice peggiore di quello che si sarebbe potuto usare se avessi accettato e compreso la bandiera.

Il problema si verifica quando le persone usano male le bandiere gettandole insieme e creando un gruppo franco flag di tutti i tipi di bandiere non correlate o cercando di usarle dove non sono affatto bandiere.

Suppongo che stiamo parlando di flag all'interno delle firme dei metodi.

L'utilizzo di una singola bandiera è già abbastanza grave.

Non significherà nulla per i tuoi colleghi nel momento in cui lo vedranno. Dovranno esaminare il codice sorgente del metodo per stabilire cosa fa. Probabilmente sarai nella stessa posizione pochi mesi dopo la linea, quando dimenticherai di cosa si trattava.

Passare un flag al metodo, normalmente significa che il tuo metodo è responsabile di più cose. All'interno del metodo probabilmente stai facendo un semplice controllo sulle linee di:

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

Questa è una scarsa separazione delle preoccupazioni e normalmente puoi trovare un modo per aggirarla.

Normalmente ho due metodi separati:

public void DoFlagSet() 
{
}

public void DoFlagNotSet()
{
}

Ciò avrà più senso con i nomi dei metodi applicabili al problema che stai risolvendo.

Il superamento di più flag è il doppio. Se davvero hai bisogno di passare più flag, piuttosto che considerarli come incapsulati all'interno di una classe. Anche allora, dovrai affrontare lo stesso problema, poiché probabilmente il tuo metodo fa più cose.

Le bandiere e la maggior parte delle variabili temporanee sono un strong odore. Molto probabilmente potrebbero essere refactored e sostituiti con metodi di query.

Revised:

Le bandiere e le variabili temporanee quando esprimono lo stato, dovrebbero essere refactored per interrogare i metodi. I valori di stato (booleani, interi e altri primitivi) dovrebbero essere quasi sempre nascosti come parte dei dettagli di implementazione.

I flag utilizzati per il controllo, il routing e il flusso generale del programma possono anche indicare l'opportunità di refactoring sezioni delle strutture di controllo in strategie o fabbriche separate, o qualsiasi cosa possa essere appropriata dal punto di vista della situazione, che continui a utilizzare i metodi di query .

Quando parliamo di flag, dovremmo sapere che verranno modificati durante il periodo di esecuzione del programma e che avranno effetto sul comportamento del programma in base ai loro stati. Finché avremo un controllo accurato su queste due cose, funzioneranno alla grande.

Le bandiere possono funzionare benissimo se

• Li hai definiti in un ambito appropriato. Con il significato appropriato, l'ambito non dovrebbe contenere codice che non è necessario / non modificare. O almeno il codice è sicuro (ad esempio non può essere chiamato direttamente dall'esterno)
• Se è necessario gestire i flag dall'esterno e se ci sono molti flag possiamo codificare il flag handler come unico modo per modificare i flag in modo sicuro. Questo flag handler può incapsulare se stesso flag e metodi per modificarli. Può essere quindi reso singleton e può quindi essere condiviso tra classi che hanno bisogno di accedere ai flag.
• E infine per la manutenibilità, se ci sono troppi flag:
  • Non c'è bisogno di dire che dovrebbero seguire nomi sensibili
  • Dovrebbe essere documentato con i valori validi (potrebbe essere con le enumerazioni)
  • Dovrebbe essere documentato con QUALE CODICE MODIFICA ciascuno di essi, e anche con WHICH CONDITION si tradurrà nell'assegnazione di un particolare valore alla bandiera.
  • QUALE CODICE VERRÀ CONSUMATO e QUALI COMPORTAMENTO determineranno un valore particolare

Se ci sono moltissime bandiere, il lavoro di progettazione dovrebbe precedere poiché le bandiere iniziano a giocare un ruolo chiave nel comportamento del programma. Puoi andare per diagrammi di stato per la modellazione. Tali diagrammi funzionano anche come documentazione e guida visiva durante il loro trattamento.

Finché queste cose sono a posto, penso che non porterà al caos.

Ho dedotto dalla domanda che il QA stava per indicare le variabili flag (globali) e non i bit di un parametro di funzione.

Ci sono situazioni in cui non hai molte altre possibilità. Ad esempio, senza un sistema operativo è necessario valutare gli interrupt. Se un'interruzione avviene molto frequentemente e non si ha il tempo di fare una lunga valutazione nell'ISR, non solo è permesso, ma a volte anche le migliori pratiche per impostare solo alcuni flag globali nell'ISR (si dovrebbe spendere il minor tempo possibile nell'ISR) e per valutare quei flag nel tuo ciclo principale.

Non credo che qualsiasi cosa sia un male assoluto nella programmazione, mai.

C'è un'altra situazione in cui le bandiere potrebbero essere in ordine, che non erano ancora state menzionate qui ...

Considera l'uso delle chiusure in questo snippet di 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 funzione interna, passata a "Array.forEach", non può semplicemente "restituire true".

Quindi, è necessario mantenere lo stato all'esterno con un flag.