0

I made a super simple design to start solving a problem.

enter image description here

Now, this might seem like super trivial at first, but since there are tons of ways to do this, it confuses me, due to my lack of professional experience.

Where would I define those compile time constants? (Always suppose I'm using the highest current C++ standard version)

In a namespace? Inside the class? In a .h outside the class? In the .cpp outside the class? Just use them as magic numbers and add some comment? static? non-static? const? constexpr? template the deck size in case its bigger?

What I thought of:

class JolloManager
{
private:
    constexpr static int rounds = 3;
    constexpr static int deckSize = 52;
    constexpr static int princeId = 1;
    constexpr static int princessId = 2;
    std::array<int, deckSize> deck;
public:
    JolloManager() {};
};

Is this correct?

Vinícius
  • 15,498
  • 3
  • 29
  • 53
  • 5
    It depends on how and where they're used. Almost any of those solutions is OK except for the "magic numbers" one. – 1201ProgramAlarm Nov 18 '19 at 16:10
  • 1
    I really wish Bathsheba didn't delete his comment, if a programmer (with his amount of rep) said something about anonymous namespace then he probably meant something – Vinícius Nov 18 '19 at 16:13
  • 1
    Note that "In a namespace? Inside a class?" is orthogonal to "In a .h outside the class? In the .cpp outside the class?". Choosing either of the first two does not help you choose from the second two. The first is about the **logical** location of the definitions; the second is about their **physical** location. – Pete Becker Nov 18 '19 at 16:16
  • There isn't a single correct answer, just a lot of tradeoffs. If you're writing something that needs to support multiple card games with different deck sizes, the implementation requirements are different than if you're writing a single-file program to learn something. Write something simple and don't over-think it - you can always change it later. – Useless Nov 18 '19 at 16:17
  • 2
    @ViníciusMagalhãesHorta: I deleted the comment since `deck` needs to know about `deskSize` due to your using `std::array`. If you were able to switch to `std::vector` then you could use an anonymous namespace in the source file implementing `JolloManager` (assuming there's only 1). At least then you're not polluting the header file. – Bathsheba Nov 18 '19 at 16:18
  • Oh, but making these constants private is possibly not ideal - since they're not mutable, you don't need to defend them from mutation, and it's probably easier to write unit tests if this kind of detail is visible. – Useless Nov 18 '19 at 16:18
  • 1
    Hi OP, do you have C++17 support? The question of how to define constants in C++ has, unfortunately, a rich history due to limitations of earlier standards. – Brian Bi Nov 18 '19 at 16:19
  • 1
    @Useless: For what it's worth I never let unit tests (the tail) wag the program (the dog). If unit tests need to access private members then you can always use the template specialisation technique (where access checks are not made). – Bathsheba Nov 18 '19 at 16:20
  • @Brian yes. I've actually edited the question. – Vinícius Nov 18 '19 at 16:22

1 Answers1

2

In C++17, defining compile-time integer constants is easy.

First, you should decide whether or not the constant should be scoped to a class. If it makes sense to have it as a class member (e.g., it pertains to the concept that the class represents) then make it a class member. Otherwise, don't.

As a class member, write:

class JolloManager {
    constexpr static int rounds = 3;
};

That's it. No out-of-line definition is required anymore in C++17.

If it's not going to be a class member, but you want everyone who includes your header to be able to access the value, then write this in the header:

inline constexpr int rounds = 3;

(Technically, the reason to use inline is to avoid ODR violations when the variable is ODR-used by an inline function in multiple translation units.)

If the value is an implementation detail that only one .cpp file needs access to, then write the following in that .cpp file to give it internal linkage (i.e., prevent clashing with names in other translation units):

constexpr int rounds = 3;  // no `inline` this time

Finally, if the constant is only needed by a single function, you can make it local to that function:

void foo() {
    constexpr int rounds = 3;
}
Brian Bi
  • 111,498
  • 10
  • 176
  • 312
  • Could you just add why would they be defined inside a namespace? – Vinícius Nov 18 '19 at 16:37
  • 2
    @ViníciusMagalhãesHorta Namespaces are really just for preventing names from clashing. In a large project, all of your code (except `main`) should be inside a namespace. – Brian Bi Nov 18 '19 at 16:38