ZActor
{
public:
//huge chunk of declarations
enum EActorState
{
ALERT,
BORED,
HAPPY,
}
//more declarations here
enum EActorType
{
MAN
WOMAN
SNAKE
}
//more declarations
}
ZDirector
{
public:
void DirectManWomanScene()
{
ZActor::EActorType eFirstActor = m_aActors[0];
ZActor::EActorState eFirstActorState = m_aActorState[0];
switch (eFirstActor])
{
case ZActor::MAN:
if (eFirstActorState == ZActor::BORED)
//do man stuff
case ZActor::WOMAN
if (eFirstActorState == ZActor::ALERT)
//prevent man from doing man stuff
}
}
private:
ZActor::EActorType m_aActors[3];
ZActor::EActorState m_aActorStates[3];
}
- And so it goes on. You will notice the annoying overuse of ZActor:: qualifier, and the cluttering of ZActor declaration with the enum, which should serve as a warning signal to anyone making or reading this type of code.
- The problem with internal enums is that typically they are so small, that people don't go thru the hassle to create a separate file for them or take them outside the class. In practice, however, enums tend to be small enough and conceptually clean that they're convenient to pass around and reuse between different classes.
- In my experience, soon after an enum is created, it's often needed in another class. Internal enums are also a bad idea esthetically, because they clutter up the class declaration.
- I've seen this again and again, where an internal enum constantly gets referenced from other classes and files, and create an absolutely unncessary dependency between the user class and the class that contains the enum.
- If your enum has a tendency to change a lot (as some enums do), then you'll find yourself recompiling unnecessary files every time you change the enum. If the enum is kept in a separate file, only the files that have the dependency on the enum will need to be recompiled.
- As a rule of thumb, I always move enums outside the class, and better yet into their own file. It is not necessary to have a separate file for each enum, but it's nice to have a separate file for a collection of enums related to the same concepts.
- Of course when the enums are outside a class, we need to make sure there are no conflicts, and it's important to use an approrpriate naming convention. Otherwise we get a lot of eUndefined, eUnlisted, eUnknown, eItemCount, eInvalid, eDefault from all over the project colliding.
The last point (conflicts) is exactly why it's often very good practice to put the enum into the class declaration. ZActor::MAN is head and shoulders above EActor_Man, if you ask me.
ReplyDeleteAnd, even if you don't, I would also dare you to come up with a scenario where moving the enum to its own file will cause less recompilation if you change it.
Go on, I'll wait ... :-)
be well
-h-
Thanks for the comment. Of course changing the enum itself will cause the same amount of recompilation whether it's in the same file or not. But ... changing the class that contains the internal enum, also forces an unnecessary recompile of all the files that have a dependency on this enum and not the class that contains it.
ReplyDeleteAs for the naming convention: it's more a matter of taste and on a case by case basis, but if your naming convention allows use of abbreviations, you shouldn't have to type stuff like
case ZActorSenseAndSensibilityManager::MAN:
and instead replace it with
case ASASM_MAN:
And we're not talking about the amount of typing here of course, but rather the amount of clutter on the screen.
1. point: You'll only recompile if the header of the enum-containing class changes. You can change the .cpp file to your heart's content without recompiling. However, if you have class A containing enum E and class B is calling class A member functions with E as an argument (which is the scenario, right?) then class B will have to to be recompiled anyway, if the header for A is changed. So, there really is no difference, as far as I can tell.
ReplyDelete2. point: Inside ZActorSenseAndSensibilityManager
you can go "case MAN:" ... how's that for reducing clutter?
1. OK, if it's the case you mention, then yes. If B has a dependency on both A and E, then there's no advantage taking it out of the class. But the case I'm talking about is where both B and A depend on E, and not on each other. At this point I realize we're splitting hairs, since it all depends on the situation and the benefits are only marginal at best. So it's a matter of marginal benefit in some situations + visual clutter.
ReplyDelete2. We can discuss this one ad-nauseum :)
3. There's also the visual clutter of having the enum inside class declarations. Again a matter of taste, but for me it hinders the readability of the class itself.
4. Plus there's the esthetic issue of having E belong to A, while in fact it conceptually can just as well belong to B or C. It has no reason, other than inertia, that E is in one file / class as opposed to another.
Note on enum visibility. I had (meaningful) name conflicts in another project and ended up with the following:
ReplyDeletenamespace Fruit { enum Enum { Banana, Apple } }
1) Enum is automatically visible in Fruit so Fruit::Banana is valid.
2) Fruit can be passed in a readable way to a function as void Foo( Fruit::Enum ).
3) If it's really obvious that Banana's only belongs to Fruit you can use 'using Fruit'.
4) You're free to declare Fruit anywhere.
So you also write programs about Apples and Bananas? High five!
ReplyDelete