0

I thought that the shuffle_string function would have generated a new seed each time gets called (so every time the cycle repeats), but something is wrong.

What could I do to circumvent this problem?

(COMPILER: g++, OS: Windows 10 x64)

Edit: Compiling with Visual Studio my code is actually running as expected.

Edit2: As suggest in the comments by @Eljay , adding 'static' before

std::random_device rd;

and before

std::mt19937_64 mt64{ rd() };

makes my program run fine even if comping with g++.

#include <iostream>
#include <random>

using std::cin; using std::cout;

void shuffle_string(std::string& s) {
    auto size = s.size();
    std::random_device rd;
    std::mt19937_64 mt64{ rd() };
    std::uniform_int_distribution<unsigned long long int> ind_range (0, size-1);
    for (auto i = 0; i < size; ++i) {
        std::swap(s[i], s[ind_range(mt64)]);
    }
}
int main() {
    while (true) {
    std::string string;
    cout << "Enter the string you want to randomly shuffle:\n";
    std::getline(cin, string);
    if (string == "exit") { return 0; }

    shuffle_string(string);

    cout << string << '\n';
    }
}

P.S.: I know i should always prefer std::shuffle, but i'm writing this only with the aim of learn something new (and actually this problem is something new).

Thanks, good evening.

thomas
  • 111
  • 8
  • you need to seed some randomness into it http://www.cplusplus.com/reference/random/mersenne_twister_engine/seed/ – pm100 Mar 19 '22 at 17:30
  • @pm100 Is that not what `mt64{ rd() };` is doing? I suppose you still would want to seed it once, not every time you call `shuffle_string`, but would that really be as bad for randomness as just seeding with the time in seconds or something that's almost guaranteed to repeat? – Nathan Pierson Mar 19 '22 at 17:33
  • 1
    What compiler are you using? Maybe it's https://stackoverflow.com/questions/18880654/ – cigien Mar 19 '22 at 17:33
  • 1
    @NathanPierson my understanding is that unseeded it will always generate the same sequence – pm100 Mar 19 '22 at 17:37
  • @cigien i'm using g++ – thomas Mar 19 '22 at 17:38
  • 1
    Make `rd` and `mt64` both `static`. – Eljay Mar 19 '22 at 18:07

1 Answers1

2

The problem is that you call the default constructor for std::random_device in this line:

std::random_device rd;

Calling the default constructor gives you a std::random_device with an implementation-defined token as source.

According to the c++ reference for std::random_device (emphasis mine)

std::random_device may be implemented in terms of an implementation-defined pseudo-random number engine if a non-deterministic source (e.g. a hardware device) is not available to the implementation. In this case each std::random_device object may generate the same number sequence.

Hence it is allowed by the standard that your call to the default constructor gives a std::random_device that always give the same series of random numbers. Next, given a initial seed for the Mersenne Twister random seed, you always get the same series of numbers. This is why your program always give the same results.

You can fix this issue by giving to the constructor of std::random_device a specific token, for example

std::random_device rd{"/dev/urandom"};

Notice that the specific tokens available are implementation-dependent.

francesco
  • 7,189
  • 7
  • 22
  • 49
  • Hello Francesco, using your correction i got this runtime error: terminate called after throwing an instance of 'std::runtime_error' what(): random_device::_M_strtoul(const std::string&) – thomas Mar 19 '22 at 17:46
  • 1
    @thomas The allowed token are implementation-dependent. Add to your question what compiler and platform you are working with. – user17732522 Mar 19 '22 at 17:50