0

I have created a function within my utility namespace to generate a random number to prevent duplicate pieces of code. Unfortunately it not as random as it re initialize the variables each time it's called, so when i loop something it gets the same output from time(0) and so the same value.

As my experience is limited i don't know how to fix it, as a singleton can't being applied due to it not being a class.

#ifndef UTILS_INT_HEADER_INCLUDED
#define UTILS_INT_HEADER_INCLUDED

#include <iostream>
#include <random>
#include <ctime>

namespace utils
{
    static int random(int min, int max, int seed) {
        std::default_random_engine generator;
        generator.seed(seed);
        std::uniform_int_distribution<int> dist(min, max);
        return dist(generator);
    }

    static int random(int max) {
        return random(0, max, time(0));
    }

    static int random(int min, int max) {
        return random(min, max, time(0));
    }
}

#endif
IMarks
  • 187
  • 11
  • 5
    Seed the generator at main or global level, not at each call. – crashmstr Oct 26 '17 at 19:02
  • That's what the `seed` argument does. What's the problem here? Don't use a crappy seed if you don't want predictable or duplicated results. Using `std::random_device` as a seed is probably a better idea than the tired, weak `time(NULL)` approach. – tadman Oct 26 '17 at 19:02
  • You want your `std::default_random_engine` to be in static scope, instead of automatic scope and having to initialize it on every call. – Sam Varshavchik Oct 26 '17 at 19:02
  • Possible duplicate of [How to generate different random numbers in a loop in C++?](https://stackoverflow.com/questions/4926622/how-to-generate-different-random-numbers-in-a-loop-in-c) – crashmstr Oct 26 '17 at 19:03
  • @crashmstr it gets similarity but i still believe the solution i require is a bit more complicated. My issue is due it to be a static method. I cannot use a initialize method where i set the seed in or use a singleton to keep the seed-lifespan alive. What do you mean by 'at main or global level'? – IMarks Oct 26 '17 at 19:07
  • You need to initialize your random number generator *once*. That is exactly the problem in the question I marked as this being a duplicate of. I'm not sure why you are declaring the functions as `static` (methods are in classes), but you should have *one* `generator` that gets initialized once and not every time the method is called (`time(0)` is not that accurate). – crashmstr Oct 26 '17 at 19:29
  • See also [C++ randomized number always same even with time(0) as seed](https://stackoverflow.com/questions/25136084/c-randomized-number-always-same-even-with-time0-as-seed?rq=1) – crashmstr Oct 26 '17 at 19:29
  • With regards to your usage of `static`: [The static keyword and its various uses in C++](https://stackoverflow.com/a/15235626/1441) (bottom of answer where it talks about static free-functions) – crashmstr Oct 26 '17 at 19:34
  • That's exactly the same as I say. if you read the question carefully, I say where the cause is, but do not know how to solve it this time due to it not being a class but a namespace function. – IMarks Oct 26 '17 at 19:34
  • I would declare the functions in the header (not as `static`, as that has no useful meaning here), and the *implementation* could do something to initialize it as needed and only once (file scoped variables, for example). Also, if you need to have multiple seeded generators at the same time (assuming you actually want callers to use the `int seed` version), then you need to have state *somewhere*, either something returned to the caller to reuse or to use a `class` to keep that state. – crashmstr Oct 26 '17 at 19:43

0 Answers0