Are flag variables an absolute evil?

Question

Are flag variables evil? Are the following kind of variables profoundly immoral and is it wicked to use them?

"boolean or integer variables that you assign a value in certain places then down below you check then in orther to do something or not, like, for example using newItem = true then some lines below if (newItem ) then"

I remember doing a couple of projects where I totally neglected using flags and ended up with better architecture/code; however, it is a common practice in other projects I work at, and when code grows and flags are added, IMHO code-spaghetti also grows.

Would you say there are any cases where using flags is a good practice or even necessary?, or would you agree that using flags in code are... red flags and should be avoided/refactored; me, I just get by with doing functions/methods that check for states in real time instead.

Answer 1

The issue I have seen when maintaining code that makes use of flags is that the number of states grows quickly, and there are almost always unhandled states. One example from my own experience: I was working on some code that had these three flags

bool capturing, processing, sending;

These three created eight states (actually, there were two other flags as well). Not all the possible value combinations were covered by the code, and users were seeing bugs:

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

It turned out there were situations where the assumption in the if statement above was false.

Flags tend to compound over time, and they hide the actual state of a class. That is why they should be avoided.

Answer 2

Here's an example when flags are useful.

I have a piece of code which generates passwords (using a cryptographically secure pseudorandom number generator). The caller of the method chooses whether or not the password should contain capital letters, small letters, digits, basic symbols, extended symbols, Greek symbols, Cyrillic ones and unicode.

With flags, calling this method is easy:

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

and it can even be simplified to:

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

Without flags, what would be the method signature?

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

called like this:

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

---

As noted in the comments, another approach would be to use a collection:

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

This is much more readable compared to the set of true and false, but still has two drawbacks:

The major drawback is that in order to allow combined values, like CharacterSet.LettersAndDigits you would be writing something like that in Generate() method:

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

possibly rewritten like this:

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

Compare this with what you have by using flags:

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

The second, very minor drawback is that it's not clear how would the method behave if called like this:

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

Answer 3

A huge function block is the smell, not the flags. If you set the flag on line 5, then only check for the flag on line 354, then that's bad. If you set the flag on line 8 and check for the flag on line 10, that's fine. Also, one or two flags per block of code is fine, 300 flags in a function is bad.

Answer 4

Usually flags can be completely replaced by some flavor of the strategy pattern, with one strategy implementation for every possible value of the flag. This makes adding new behavior a lot easier.

In performance critical situations the cost of the indirection might surface and make deconstruction into clear flags necessary. That being said I'm having trouble to remember a single case where I actually had to do that.

Answer 5

No, flags are not bad or an evil that must be refactored away at all costs.

Consider Java's Pattern.compile(String regex, int flags) call. This is a traditional bitmask and it works. Glance at the constants in java and wherever you see a bunch of 2ⁿ you know there are flags being there.

In an ideal refactored world, one would instead use an EnumSet where the constants are instead values in an enum and as the documentation reads:

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 a perfect world, that Pattern.compile call becomes Pattern.compile(String regex, EnumSet<PatternFlagEnum> flags).

All that said, its still flags. It is much easier to work with Pattern.compile("foo", Pattern.CASE_INSENSTIVE | Pattern.MULTILINE) than it would be to have Pattern.compile("foo", new PatternFlags().caseInsenstive().multiline()) or some other style of trying to do what flags really are and good for.

Flags are often seen when working with system level things. When interfacing with something at the operating system level, one is likely to have a flag somewhere - be it the return value of a process, or the permissions of a file, or the flags for opening a socket. Trying to refactor these instances away in some witch hunt against a perceived code smell will likely end up with worse code than if one used accepted and understood the flag.

The problem occurs when people misuse flags tossing them together and creating a frankenflag set of all sorts of unrelated flags or trying to use them where they aren't flags at all.

Answer 6

I'm assuming we are talking about flags within method signatures.

Using a single flag is bad enough.

It will mean nothing to your colleagues the firs time they see it. They'll have to look at the source code of the method in order to establish what it does. You will probably be in the same position few months down the line, when you forget what your method was all about.

Passing a flag to the method, normally means that your method is responsible for multiple things. Inside the method you are probably doing a simple check on the lines of:

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

That's a poor separation of concerns and you can normally find a way around it.

I normally have two separate methods:

public void DoFlagSet() 
{
}

public void DoFlagNotSet()
{
}

This will make more sense with method names that are applicable to the problem that you are solving.

Passing multiple flags is twice as bad. If you really do need to pass multiple flags, than consider encapsulating them within a class. Even then, you will be still facing the same problem, as your method is likely doing multiple things.

Answer 7

Flags and most temp variables are a strong smell. Most likely they could be refactored and replaced with query methods.

Revised:

Flags and temp variables when expressing state, should be refactored to query methods. The state values (booleans, ints, and other primatives) should almost -always- be hidden as part of the implementation details.

Flags that are used for control, routing, and general program flow may also indicate the opportunity to refactor sections of the control structures into seperate strategies or factories, or whatever might be situationally appropriate, that continue to make use of the query methods.

Answer 8

When we talk about flags we should know that they are going to get modified over the time of program execution and that they are going to effect the behavior of program based on their states. As long as we have neat control over these two things, they will work great.

Flags can work great if

• You have defined them in an appropriate scope. By appropriate I mean the scope should not contain any code that need-not/should-not modify them. Or at least the code is secure (for example it may not be called directly from outside)
• If there is need to handle flags from outside and if there are many flags we can code flag handler as a only way to safely modify flags. This flag handler may itself encapsulate flags and methods to modify them. It can be then made singleton and can be then shared among classes that needs access to flags.
• And finally for maintainability, if there are too many flags:
  • No need to say they should follow sensible naming
  • Should be documented with what are valid values (may be with enumerations)
  • Should be documented with WHICH CODE WILL MODIFY each of them, and also with WHICH CONDITION will result in assignment of a particular value to the flag.
  • WHICH CODE WILL CONSUME them and WHAT BEHAVIOR will result for a particular value

If there are hell lot of flags good design work should precede since flags then start playing key-role in program behavior. You can go for State diagrams for modeling. Such diagrams also work as documentation and visual guidance while dealing with them.

As long as these things are in place I think it will not lead to the mess.

Answer 9

I assumed from the question that the QA was meaning flag (global) variables, and not bits of a function parameter.

There are situations where you don't have much other possibilities. For example, without an operating system you have to evaluate interrupts. If an interrupt comes very frequently and you don't have time to do a lengthy evaluation in the ISR, it is not only allowed but sometimes even best practice to only set some global flags in the ISR (you should spend as little time as possible in the ISR), and to evaluate those flags in your main loop.

Answer 10

I don't think anything is an absolute evil in programming, ever.

There is another situation where flags might be in order, which were not mentioned here yet...

Consider the use of closures in this Javascript snippet:

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

The inner function, being passed to the "Array.forEach", cannot simply "return true".

Hence, you need to keep the state outside with a flag.