How to make a roll_die() function with an optional random seed parameter

Question

New to c++. I'd like to make a function like roll_die(unsigned int seed = ???) where, by default the function uses srand(time(NULL)) to seed the RNG before returning a value, but also allows the user to specify a seed. Here's what I have that works

#include <iostream>
#include <ctime>

int roll_die(unsigned int seed = 0){
    // Returns a random integer between 1 and 6
    // time(NULL) is the number of seconds since 00:00 hours, Jan 1, 1970 UTC

    // Logic
    srand(seed);
    int randnum = rand();

    // Print what's going on
    std::cout << "The random seed is: " << seed << std::endl;
    std::cout << "The max random number is: " << RAND_MAX << std::endl;
    std::cout << "The randomly chosen number is: " << randnum <<std::endl;

    // Return the result
    int result = randnum % 6 + 1;
    return result;
}

This is nice, but it doesn't default to a random seed. If I try doing something like int roll_die(unsigned int seed = time(NULL)){...} I get the warning

Unable to parse C++ default value 'time(NULL)' for argument seed of function roll_die

I assume because time(NULL) does not return an int.

What is the proper way to do this?

@Slava While the dupe is something the OP is going to have to deal with, it isn't what the question is asking about. They have 2 issues and are asking about issue 1 while you have closed it for issue 2. I'm not sure if it should be closed as is — NathanOliver, Sep 27 '18 at 17:31
@NathanOliver I think it covers all issues OP has - this function should not call srand() at all. Proper way is to call `srand()` in main and dup perfectly explains it. — Slava, Sep 27 '18 at 17:32
Thanks @NathanOliver - I also don't see how the linked "duplicate" question is actually a duplicate of what I'm asking here. At least, it's not obvious to me. — Ben, Sep 27 '18 at 17:33
@Slava I've asked the OP to post the warning. I don't see anything in the dupe that talks about getting a warning from `unsigned int seed = time(NULL)` — NathanOliver, Sep 27 '18 at 17:35
@Ben your function that returns random number should not call srand() at all and dup explains why. — Slava, Sep 27 '18 at 17:35
@NathanOliver because `std::time()` returns `std::time_t` not `unsigned int` — Slava, Sep 27 '18 at 17:36
`time` returns a `time_t`. I've never seen a `time_t` that isn't an integer type, probably not even legal to be anything but an integer type, so you can cast the return value from of `time` to an `unsigned int` — user4581301, Sep 27 '18 at 17:36
But it's important to note that `time`'s granularity is 1 second, so if you reseed twice in one second you will get the same sequence. Probably not what you want and similar to what Slava's warning about. — user4581301, Sep 27 '18 at 17:38
@NathanOliver the question is "What is the proper way to do this?" dupe answers - do not call `srand()` from this function, this is a proper way. If you disagree fill free to vote for reopen. — Slava, Sep 27 '18 at 17:39
@Ben What compiler are you using? I can't reproduce with [g++](http://coliru.stacked-crooked.com/a/765b447ff38d5421) or [clang](http://coliru.stacked-crooked.com/a/259efe170c25f48c) — NathanOliver, Sep 27 '18 at 17:42
I'd suggest *not* using `srand`/`rand` at all and instead use the much nicer modern [](https://en.cppreference.com/w/cpp/numeric/random) facilities. — Jesper Juhl, Sep 27 '18 at 17:42
I believe my compiler is `Apple LLVM version 10.0.0 (clang-1000.11.45.2)` from running `c++ --version` in terminal. — Ben, Sep 27 '18 at 17:45
I think the question [Converting time_t to int](https://stackoverflow.com/questions/9483974/converting-time-t-to-int) is really close to what you want. — user4581301, Sep 27 '18 at 17:49
THere's something to be said about just telling the asker what they want and not pointing them to "better" random functions, or arguing about the resolution of the timers, of opening/closing the question. :-) — Jeffrey, Sep 27 '18 at 18:09
With you part way on that @Jeffrey . Remember that SO is a repository of solutions to programming problems. If you give the asker what they want and what they want is a really bad idea, you owe it to the people who come after to explain why it's a horrible idea and present better solutions. Slava is dead right that this is a horrible idea, even if this doesn't do Ben any good. — user4581301, Sep 27 '18 at 18:15

Yakk - Adam Nevraumont · Answer 1 · 2018-09-27T18:45:37.593

int roll_die(unsigned int seed = 0){
  srand(seed);

this is code smell.

You don't srand every time you roll a die.

You are suppose to seed your random number generator, then generate a stream of random numbers from it.

The modern C++ way to do this is they split random value generation into generators and distributions.

Generators are where the entropy is, and produce bits.

Distributions consume Generators and produce random values.

The C approach of srand and rand leads to problems in large programs. Some of these problems are common to "global state" functions (threading, interference between two pieces of code who use the engine, etc), and some are due to the poor quality implementations many srand and rand functions are backed with.

int roll_die(
  std::mt19937& engine
){
  auto rand = [&engine]{
    return std::uniform_int_distribution<>{1,6}(engine);
  };
  int result = rand();
  return result;
}
// very inefficient
int roll_die() {
  std::random_device d;
  std::mt19937 engine(d());
  return roll_die(engine);
}

note that I don't use %6 to generate a value; that doesn't generate uniform values.

I would advise against the zero-overload version of roll_die. Going to a std::random_device on every roll is expensive. Using something worse (like the current time in seconds) is worse, as two rolls in the same second give the same die roll.

Really, you should do one random_device somewhere, use it to seed your std::mt19937 and reuse the std::mt19937 for the lifetime of your program/simulation/etc.

You should also record the seed used and provide options to input a seed. That will let you create repeatable simulations and reproduce bugs better.

Jeffrey · Accepted Answer · 2018-09-27T20:24:17.073

0

What you want is:

void roll_die(unsigned int seed = static_cast<unsigned int>(time(NULL)))

Basically, you just need to tell the compiler to cast the time_t into the proper time.

However, do take into account the need to not seed at every throw. See other answer.

edited Sep 27 '18 at 20:24

answered Sep 27 '18 at 18:07

Jeffrey

11,063
1
21
42

Bingo. (Props to @user4581301 for pointing this out in the comments.) – Ben Sep 27 '18 at 18:13

How to make a roll_die() function with an optional random seed parameter

2 Answers2