8

I have some code that looks like this:

// Player.hpp
class Player
{
public: enum class State { RUN, WALK, STAND };
};

The enum class State is used by the Player class, as well as by many other external components. This way every other component that needs the Player::State definition has to include the Player.hpp file. I'm not 100% sure about this solution.

Another option would be to put the state in a separate "types.hpp" file, like so:

// types.hpp
enum class PlayerState { RUN, WALK, STAND };

What are in your opinion the pros and the cons of these designs?

Improve this question

3 Answers 3

Reset to default
9

The primary reasons to put a type into a class/struct is for template metaprogramming, or if the type is private to the class. Otherwise, just put it next to the class:

// Player.hpp
...

enum class PlayerState { RUN, WALK, STAND };

class Player {
public:
  ...
private:
   PlayerState m_state;
};

In C++, it is common that you will have some helper declarations for a class. So it is not reasonable to enforce a one declaration per header policy.

Putting this declaration in a separate file can be sensible if the PlayerState is used separately from the Player. But given these names, that's unlikely to be the case.

Separate headers are especially helpful when the header needs to include additional headers for some declarations. Because C++ (currently) has no module systems, the contents of those headers also become available to whoever includes the header you are writing – an impact we would like to minimize. An alternative solution to these header dependencies is to abstract over details of your class definitions, e.g. using polymorphism (the header declares only an interface whereas the implementation classes are internal to a .cpp file), or using the pImpl idiom.

Improve this answer
2
  • Did you mean to remove the enums public access modifier? Commented May 20, 2018 at 13:48
  • 1
    @CandiedOrange In C++, visibility only affects class/struct members. Declarations in namespaces (or the top level) are always public. To have private declarations in a header, by convention they are put into a nested namespace detail or similar. Commented May 20, 2018 at 13:51
4

This looks like a case where you want namespaces. They're the natural way in C++ to group related names.

Namespaces, unlike classes, can be reopened. That means you can have multiple headers all adding to the same namespace. Just look at all the headers that contribute to namespace std

Improve this answer
-1

Sometimes a good practice is to have a "Defs.h" file which will have the emuns and constants that are used by several classes, which those classes will include.

Improve this answer
3
  • 1
    Disagree completely. Definitions should go where there are used. PlayerState should go in Players.hpp because it is only used by the Player class. If you have definitions that are used thru-out your app, they should go in AppDefs.hpp. Commented May 20, 2018 at 12:35
  • @shawnhcorey, that's what I've meant by "used by several classes". your last sentence is what I wrote, you "disagree completely" (drama) with something I didn't even wrote. Commented May 20, 2018 at 23:09
  • The states of a state machine will only be used by a single class and its derived classes. To place the identifiers in a file not associated with the class is to break encapsulation. Commented May 21, 2018 at 8:03

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.