2
votes

My goal is to have a simple program with dice of different number of sides, each with its own random engine and distribution. My code generates random numbers but they are all huge and the same numbers are generated for different sided dice. My constructor contains std::mt19937, std::uniform_int_distribution, a seed, and a number of sides which is passed in. With everything in the constructor, it won't work, but if I put the seed, random engine and distribution as static members I can get it to work, but then I have only one distribution for all of my dice.

// Die.h
#pragma once

#include <random>

class Die {
   private:
      int numSides;
      long int seed;
      std::mt19937_64 randomEngine;
      std::uniform_int_distribution<int> dieDist;
   public:
      explicit Die(int numSides);
      int roll();
};

// Die.cpp
#include "Die.h"
#include <ctime>
#include <iostream>

Die::Die(int numSides) : numSides(numSides) {
   seed = static_cast<long int>(std::time(nullptr));
   std::mt19937_64 randomEngine(seed);
   std::uniform_int_distribution<int> dieDist(1,numSides);
}

int Die::roll() {
   return dieDist(randomEngine);
}

// Die.h testing
#include <iostream>
#include "Die.h"

int main() {
   Die side4Die(4);
   Die side6Die(6);
   Die side8Die(8);
   Die side10Die(10);
   Die side12Die(12);
   Die side20Die(20);

   for (int i = 0; i < 20; i++) {
      std::cout << side4Die.roll() << "  ";
   }
   std::cout << "\n\n";

   for (int i = 0; i < 20; i++) {
      std::cout << side6Die.roll() << "  ";
   }
   std::cout << "\n\n";

   for (int i = 0; i < 20; i++) {
      std::cout << side8Die.roll() << "  ";
   }
   std::cout << "\n\n";
}

What I expected to see was output of 20 numbers 1 through 4, 20 numbers 1 through 6, then 20 numbers 1 through 8. Can someone explain how to declare the random generator and uniform distribution in the header file and then how to define them in the implementation file? What I get as output is three lines of

1689685134 537902435 1526154843 2032953622 41384282 869520735 539700904 48774590 1118072656 740173846 588830575 1204807261 300732443 1167922011 1120805453 1840559451 1073257265 900590269 1598330246 535084483

1

1 Answers

2
votes

Your Die constructor is declaring two local variables, randomEngine and dieDist, and these override (or hide) the class members with the same names - and those are thus left unitialized.

Instead, to set up the class members, use code like this:

Die::Die(int nSides) : numSides(nSides) // Best not to use the same name twice!
{
    seed = static_cast<long int>(std::time(nullptr));
    randomEngine = std::mt19937_64(size_t(seed)); // Argument really should be a size type.
    dieDist = std::uniform_int_distribution<int>(1, numSides);
}

Feel free to ask for further clarification and/or explanation.

Note: Turning on compiler warnings can help spot problems like this! For your original code, clang-cl gives several of these:

warning : declaration shadows a field of 'Die' [-Wshadow]