Is it a bad practice to include all the enums in one file and use it in multiple classes?

Question

I'm an aspiring game developer, I work on occasional indie games, and for a while I've been doing something which seemed like a bad practice at first, but I really want to get an answer from some experienced programmers here.

Let's say I have a file called enumList.h where I declare all the enums I want to use in my game:

// enumList.h

enum materials_t { WOOD, STONE, ETC };
enum entity_t { PLAYER, MONSTER };
enum map_t { 2D, 3D };
// and so on.

// Tile.h
#include "enumList.h"
#include <vector>

class tile
{
    // stuff
};

The main idea is that I declare all enums in the game in 1 file, and then import that file when I need to use a certain enum from it, rather than declaring it in the file where I need to use it. I do this because it makes things clean, I can access every enum in 1 place rather than having pages openned solely for accessing one enum.

Is this a bad practice and can it affect performance in any way?

Answer 1

I really think it is a bad practice. When we measure code quality, there's something we call "granularity". Your granularity suffers severely by putting all those enums in one single file and thus maintainability suffers, too.

I'd have a single file per enum to find it quickly and group it with the behavioural code of the specific functionality (e.g. materials enum in folder where material behaviour is etc.);

The main idea is that I declare all enums in the game in 1 file, and then import that file when I need to use a certain enum from it, rather than declaring it in the file where I need to use it. I do this because it makes things clean, I can access every enum in 1 place rather than having pages openned solely for accessing one enum.

You might think it is clean, but in fact, it is not. It's coupling things that do not belong together functionality- and module-wise and decreases the modularity of your application. Depending on the size of your code base and how modular you want your code to be structured, this might evolve into a bigger problem and unclean code/dependencies in other parts of your system. However, if you just write a small, monolithic system, this does not necessarily apply. Yet, I would not do it like this even for a small monolithic system.

Answer 2

Yes, it's bad practice, not because of performance but because of maintainability.

It makes things "clean" only in the OCD "collect similar things together" way. But that's actually not the useful and good kind of "cleanliness".

Code entities should be grouped to maximize cohesion and minimize coupling, which is best achieved by grouping them into largely independant functional modules. Grouping them by technical criteria (such as putting all enums together) achieves the opposite - it couples code that has absolutely no functional relatioship and puts enums that might be used only in one place into a different file.

Answer 3

Well, this is just data (not behaviour). The worst that could possibly/theoretically happen is that the same code is included and compiled more than once producing a relatively larger program.

It is just plain impossible that such includes can add more cycles to your operations, given that there isn't any behavioural/procedural code inside them (no loops, no ifs, etc.).

A (relatively) larger program can hardly affect performances (speed of execution) and, in any case, is just a remote theoretical concern. Most (maybe all) compilers manage includes in a way that prevent such problems.

IMHO, the advantages that you get with a single file (more readable and more manageable code) largely exceed any possible drawback.

Answer 4

To me it all depends on the scope of your project. If there is one file with say 10 structs and that are the only ones ever used, I'd be perfectly comfortable having one .h file for it. If you have several different types of functionality, say a units, economy, buildings, etc I would definitely split things up. Create a units.h where all structs that deal with units reside. If you want to do something with units somewhere you need to include units.h sure, but it also is a nice "identifier" that something with units is probably done in this file.

Look at it this way, you don't buy a supermarket because you need a can of coke ;)

Answer 5

I'm not a C++ dev, so this answer will be more generic to OOA&D:

Typically, code objects should be grouped in terms of functional relevance, not necessarily in terms of language-specific constructs. The key yes-no question that should always be asked is, "will the end coder be expected to use most or all of the objects that they get when consuming a library?" If yes, group away. If not, consider splitting the code objects up and placing them closer to other objects that need them (thereby increasing the chance that the consumer will need everything they are actually accessing).

The basic concept is "high cohesion"; code members (from methods of a class all the way up to classes in a namespace or DLL, and DLLs themselves) should be organized such that a coder can include everything they need, and nothing they don't. This makes the overall design more tolerant of change; things that must change can be, without affecting other code objects that did not have to change. In many circumstances it also makes the application more memory-efficient; a DLL is loaded in its entirety into memory, regardless of how much of those instructions are ever executed by the process. Designing "lean" applications therefore requires paying attention to how much code is pulled into memory.

This concept applies at virtually all levels of code organization, with varying degrees of impact on maintainability, memory-efficiency, performance, build speed, etc. If a coder needs to reference a header/DLL of rather monolithic size just to access one object, which doesn't depend on any other object in that DLL, then that object's inclusion in the DLL should probably be rethought. However it is possible to go too far the other way as well; a DLL for every class is a bad idea, because it slows build speed (more DLLs to rebuild with associated overhead) and makes versioning a nightmare.

Case in point: if any real-world use of your code library would involve using most or all of the enumerations that you're putting into this single "enumerations.h" file, then by all means group them together; you'll know where to look to find them. But if your consuming coder could conceivably need only one or two of the dozen enumerations you are providing in the header, than I would encourage you to put those into a separate library and make it a dependency of the larger one with the rest of the enumerations. That allows your coders to get just the one or two they want without linking to the more monolithic DLL.

Answer 6

This sort of thing becomes an issue when you have multiple developers working on the same code-base.

I have certainly seen similar global files becoming a nexus for merge collisions and all sorts of grief on (larger) projects.

However, if you are the only one working on the project, then you should do whatever you are most comfortable with, and only adopt "best practices" if you understand (and agree with) the motivation behind them.

It is better to make some mistakes and learn from them than to risk getting stuck with cargo cult programming practices for the rest of your life.

Answer 7

Yes, it is bad practice to do so on a big project. KISS.

Young coworker renamed a simple variable in a core .h file and 100 engineers waited 45 minutes for all their files to rebuild, which affected everyone's performance. ;)

All projects start small, balloon over the years, and we curse those who took early shortcuts for creating technical debt. Best practices, keep global .h content limited to what's necessarily global.

Answer 8

In this case, I would say the ideal answer is that it depends on how the enums are consumed, but that in most circumstances it's probably best to define all enums separately, but if any of them are already coupled by design, you should provide a means of introducing said coupled enums collectively. In effect, you have a coupling tolerance up to the amount of intentional coupling already present, but no more.

Considering this, the most flexible solution is likely to define each enum in a separate file, but provide coupled packages when it's reasonable to do so (as determined by the intended usage of the enums involved).

---

Defining all of your enumerations in the same file couples them together, and by extension causes any code that depends on one or more enums to depend on all enums, regardless of whether the code actually uses any other enums.

#include "enumList.h"

// Draw map texture.  Requires map_t.
// Not responsible for rendering entities, so doesn't require other enums.
// Introduces two unnecessary couplings.
void renderMap(map_t, mapIndex);

renderMap() would much rather only know about map_t, because otherwise any changes to the others will affect it even though it doesn't actually interact with the others.

#include "mapEnum.h" // Theoretical file defining map_t.

void renderMap(map_t, mapIndex);

---

However, in the case where components are already coupled together, providing multiple enums in a single package can easily provide additional clarity and simplicity, provided that there's a clear logical reason for the enums to be coupled, that usage of those enums is also coupled, and that providing them doesn't also introduce any additional couplings.

#include "entityEnum.h"    // Theoretical file defining entity_t.
#include "materialsEnum.h" // Theoretical file defining materials_t.

// Can entity break the specified material?
bool canBreakMaterial(entity_t, materials_t);

In this case, there's no direct, logical connection between entity type and material type (assuming that the entities aren't made of one of the defined materials). If, however, we had a case where, e.g., one enum is explicitly dependent on the other, then it makes sense to provide a single package containing all coupled enums (as well as any other coupled components), so that the coupling can be isolated to that package as much as is reasonably possible.

// File: "actionEnums.h"

enum action_t { ATTACK, DEFEND, SKILL, ITEM };               // Action type.
enum skill_t  { DAMAGE, HEAL, BUFF, DEBUFF, INFLICT, NONE }; // Skill subtype.

// -----

#include "actionTypes.h" // Provides action_t & skill_t from "actionEnums.h", and class Action (which couples them).
#include "entityEnum.h"  // Theoretical file defining entity_t.

// Assume ActFlags is or acts as a table of flags indicating what is and isn't allowable, based on entity_t and Action.
ImplementationDetail ActFlags;

// Indicate whether a given type of entity can perform the specified action type.
// Assume class Action provides members type() and subtype(), corresponding to action_t and skill_t respectively.
// Is only slightly aware of the coupling; knows type() and subtype() are coupled, but not how or why they're coupled.
bool canAct(entity_t e, const Action& act) {
    return ActFlags[e][act.type()][act.subtype()];
}

But alas... even when two enums are intrinsically coupled together, even if it's something as strong as "second enum provides subcategories for first enum", there may still come a time where only one of the enums is necessary.

#include "actionEnums.h"

// Indicates whether a skill can be used from the menu screen, based on the skill's type.
// Isn't concerned with other action types, thus doesn't need to be coupled to them.
bool skillUsableOnMenu(skill_t);

// -----
// Or...
// -----

#include "actionEnums.h"
#include "gameModeEnum.h" // Defines enum gameMode_t, which includes MENU, CUTSCENE, FIELD, and BATTLE.

// Used to grey out blocked actions types, and render them unselectable.
// All actions are blocked in cutscene, or allowed in battle/on field.
// Skill and item usage is allowed in menu.  Individual skills will be checked on attempted use.
// Isn't concerned with specific types of skills, only with broad categories.
bool actionBlockedByGameMode(gameMode_t mode, action_t act) {
    if (mode == CUTSCENE) { return true; }
    if (mode == MENU) { return (act == SKILL || act == ITEM); }

    //assert(mode == BATTLE || mode == FIELD);
    return false;
}

---

Therefore, since we know both that there can always be situations where defining multiple enumerations in a single file can add unnecessary coupling, and that providing coupled enums in a single package can clarify the intended usage and allow us to isolate the actual coupling code itself as much as possible, the ideal solution is to define each enumeration separately, and provide joint packages for any enums that are intended to frequently be used together. The only enums defined in the same file will be ones that are intrinsically linked together, such that usage of one necessitates usage of the other as well.

// File: "materialsEnum.h"
enum materials_t { WOOD, STONE, ETC };

// -----

// File: "entityEnum.h"
enum entity_t { PLAYER, MONSTER };

// -----

// File: "mapEnum.h"
enum map_t { 2D, 3D };

// -----

// File: "actionTypesEnum.h"
enum action_t { ATTACK, DEFEND, SKILL, ITEM };

// -----

// File: "skillTypesEnum.h"
enum skill_t  { DAMAGE, HEAL, BUFF, DEBUFF, INFLICT, NONE };

// -----

// File: "actionEnums.h"
#include "actionTypesEnum.h"
#include "skillTypesEnum.h"