Flags are a code smell

In most major code bases, it’s common to have objects with lots of Boolean flags indicating what state they’re in. E.g. is an enemy targeting the player or not? Is it trying to flee or not? If you have a set of mutually exclusive flags an enum can be used instead, but this is often not possible, and doesn’t change much in terms of the downsides.

In my opinion these are code smells. Smells don’t necessarily indicate actual issues, just patterns that are likely to be used incorrectly and thus warrants extra care.

In many cases if you think about how to avoid flags you’re forced change the code in ways that result in clearer code (specifically several simple functions rather than one general function that checks a dozen flags), as well as improved performance due to fewer branches and cache misses. Let me stress that the main gain here is the improved code quality, so don’t be fooled into thinking this is some kind of optimization – better performance is a happy side effect. And yes, there are cases where flags are the clearer option as well (see below).

An example

For a somewhat contrived example, consider a game rendering system. Each object in the world has some rendering state associated with it, this would be things like what meshes and materials to use, but also various flags determining how it’s to be rendered.

Let’s say we have a flag called isDynamic, which tells us if the object can move or if it’s static. We can change the flag throughout the life of the object, but when it’s set to false we assume it’s going to be static for some time (so we pay some up-front cost to pre-calculate some acceleration structures that pay off during actual rendering).

Sometimes you want to temporarily toggle rendering for an object so we check this in the renderer as well by adding an isVisible flag.

In addition, the object as a whole may have an isEnabled flag which lets us disable the whole object (physics, animation, rendering all ignore it when it’s not enabled).

Ok, so far we have three flags which means we have 23 = 8 possible combinations of flags, but do we really have eight separate legal states? No! E.g. an object which is not enabled can’t be visible. You could also argue that an object which is disabled or invisible is _neither _dynamic nor static. So we have significantly more possible states than legal states, and it’s very easy to accidentally end up in an illegal state by forgetting to check a flag in one of the many branches throughout the renderer (e.g. perhaps you go down the “isDynamic” branch somewhere for an object which is invisible, thus getting yourself into an inconsistent state). As you add more flags the set of possible states explodes, and it’s common for the interdependencies to be complex and easy to mess up in all the places you have to check.

An alternative

Instead of storing all these flags, take a step back and think about what the point of each flag is. The isDynamic flag is there to distinguish between dynamic and static objects. A different way of doing this is for the renderer to simply keep an array of dynamic objects and an array of static objects.

The isVisible flag is different. It means the object should be in neither the dynamic or static array. For the whole-object enabled flag we can do something similar, but for each sub-system.

The exact way you avoid any given flag will differ depending on what the purpose of the flag is, but you can usually find a way to avoid it._ _

So that’s the point of all this? Well the main benefit is that all the code spread out through the renderer can now be simpler. Instead of checking flags for a complicated set of legal states all over the code, we just have completely separate functions for each kind of state. E.g. there’s one method to go through all the static objects and render them, another to go through all the dynamic ones. These functions may call into shared helper functions here and there (to avoid duplication), but if you want to know how dynamic objects get rendered there’s a single place to start reading, and it will be easier to understand because the code is handling fewer cases.

The complexity isn’t entirely gone, it’s just moved. It’s now in methods like “SetVisible”, “SetDynamic” and so on. This is far better, since the complexity is now localized, making it easier to ensure correctness.

A brief note on performance: As we’re now separating things into different arrays to keep the logic clearer, you can often also split the data up across different arrays, which improves the cache locality for the functions that deal with that particular part of the state. For example, objects may use different data if they’re static or dynamic so if you can avoiding have to “step over” the static data in the dynamic code-path that’s going to save lots of cache misses.

What about checking which state an object is in?

If you use the proposed scheme most code never actually have to check any per-object flags, because the data already comes pre-sorted to the “processing” code (and is kept sorted by the state transition code). However, there are exceptions. Sometimes you really do want to check what state a specific object is in. This means doing an array lookup for each of the different arrays to see if the object is there.

To make this fast, you could store the index for each array in the object, which gives you fast lookups. This does means you’re still storing state-related information in the object itself (one index per array), but it can be private information so you still get the other benefits - the renderer still doesn’t have to check flags.

Another option is to make sure each object uses the same index in all the arrays it’s in. You generate a unique ID for each object, and use this as the location for the object in all arrays. The obvious way to do this is to allocate MAX_OBJECT_COUNT-sized arrays for all possible states, but that can of course be wasteful (and costly - gaps in the array cost cache misses when traversing). A better way is to use a “sparse array”, which only stores objects that are actually in the array, but still allows O(1) lookup. A hash table is a reasonable backing-store for this, but you can do better if you let your IDs be unique ints with uniform distribution (no need to actually hash anything, use the ID as-is in a Robin-hood style hash table).

When not to use this

I said above that flags are a code smell, not code bugs! There’s plenty of cases where using flags is entirely appropriate. E.g. if you set flags multiple times per frame then the increased cost of moving it between different arrays all the time is likely a bad idea. Also, if you use the sparse array idea above checking a flag is a lot more expensive so if you do this frequently you should probably at least cache the result of this lookup in a flag. Another scenario is if you only have one of something, there’s really no reason to have multiple arrays in that case, since most arrays will be empty. Also, sometimes the added cognitive overhead of multiple arrays just isn’t worth it if the code is trivial. Last but not least, sometimes it’s just damn difficult to figure out how to avoid a flag and spending too much effort jumping through hoops to satisfy some rule of thumb is just dogmatic.

Even if you go with the suggested approach above, you may still need an actual flag to handle the state transitions appropriately. E.g. if you make an object visible after it’s been invisible you probably need to know if you should add it to the dynamic or static array now that it’s visible. The point isn’t to have no flags at all (despite the title), it’s to move the resulting complexity to the code that actually deals with the _state transition_s, instead of spreading (and duplicating) that same complexity all over the code.

More resources

If this approach appeals to you, you really should look into data-oriented design. I’m a fan of this approach (first and foremost consider the data, and how it gets processed, don’t obsess over object identity), but I would advocate moderation. There are often benefits to having some of the state with the object, instead of having everything be implicit based on what component arrays it’s in (in terms of readability, and sometimes even performance). Resist the temptation to be an extremist about this, that’s really no different than the programmer who uses design patterns every chance they get.

For more on data oriented design, check out http://www.dataorienteddesign.com/dodmain/ .

Another good intro to this and other things, from a performance perspective is “Code Clinic: How to Write Code the Compiler can Actually Optimize” by Mike Acton at GDC 2014 (available at the GDC vault with membership – you really need to see the talk here though, slides alone aren’t enough).

Another practical case study is Pitfalls of Object Oriented Programming by Tony Albrecht.