Random number generator for Sally

[annolliwohe]

Hello I would like to make a function that generates random numbers between a minimum and a maximum number for my code that I posted there:
http://www.cplusplus.com/forum/beginner/272924/

void Sally::doRnd(Sally *Sptr) {
   Token p;

   if ( Sptr->params.size() < 1 ) {
      throw out_of_range("Need one parameters for RND") ;
   }
   
   p = Sptr->params.top() ;
   Sptr->params.pop() ;
   srand(time(NULL));
	 double answer = p.m_value * rand() / RAND_MAX -1;
   Sptr->params.push( Token(DOUBLE, answer, "") ) ;	       // if negative number throw an exception?
}

I made one with only one parameter (see above), I would like to do it again with two but when I try to adapt some code like this one it gives me strange results (this does not work) :

  void Sally::doRnd(Sally *Sptr) {
	std::random_device rd;
	Token p1, p2;

	if (Sptr->params.size() < 2)
	{
		throw out_of_range("Need two parameters for RND.");
	}
	p1 = Sptr->params.top();
	Sptr->params.pop();
	p2 = Sptr->params.top();
	Sptr->params.pop();
	  
    std::mt19937::result_type seed = rd() ^ (
            (std::mt19937::result_type)
            std::chrono::duration_cast<std::chrono::seconds>(
                std::chrono::system_clock::now().time_since_epoch()
                ).count() +
            (std::mt19937::result_type)
            std::chrono::duration_cast<std::chrono::microseconds>(
                std::chrono::high_resolution_clock::now().time_since_epoch()
                ).count() );

    std::mt19937 gen(seed);
    
    int min_rand = (int) p1.m_value;
    int max_rand = (int) p2.m_value;
    std::uniform_int_distribution<unsigned> distrib(min_rand, max_rand);
	double answer = double(distrib(gen));
    Sptr->params.push(Token(DOUBLE, answer, ""));
}

Could you please help me to correct my mistakes ? Thank you in advance.

[TomCPP]

it gives me strange results (this does not work)

What does it mean?
- compile error
- some crash of your application
- improper value returned from distrib(gen)
- or something else

By the way, your random value generator may be simplified to:

std::mt19937 gen(std::chrono::system_clock::now().time_since_epoch().count());
std::uniform_int_distribution<int> distrib {min_rand,max_rand};
auto answer = distrib(gen);

[annolliwohe]

Hello, I tried with your simplified code, but it gives me the same kind of result. It doesn't give me integers but results like this: 1.92441e+009.

I don't understand why?

[ne555]

¿what's a `Sally'? ¿what's its purpose? ¿what operations may you do?
¿what does your member function ask for another Sally object instead of operating on this?

Sally s;
s.doRnd(); // instead of s.doRnd(&s); 

separate the «parse» from the functions
int random(int min, int max); code and test that function, then call it from your Sally thing

> It doesn't give me integers but results like this: 1.92441e+009.
¿how are you checking that?
with a debugger stop one after the double answer = double(distrib(gen)); line, ¿what's the value of `answer'?

[jonnin]

1.92441e+009
^^ This could be an integer. This is scientific notation, c++ style. That means move the decimal 9 places .. without seeing all the digits, can't say if its a 64 bit random value or a double at all.

[seeplus]

You are mixing signed and unsigned. What are the values of min_rand and max_rand? If they are negative, then you'll get a large positive value produced by distrib() as this is returning unsigned. A large positive integer converted to double and then displayed would be in scientific notation. It is 1,924,410,000 as an integer rounder to 6 sig figs. I suggest you work in int. With the latest spec of C++ (works in the current version of VS), you don't even need to specify the type for uniform_int_distribution as the type is deduced.

As an example, try this:

#include <random>
#include <iostream>
#include <chrono>

int main()
{
	const int min_rand = -50;
	const int max_rand = 500;

	std::mt19937 gen(std::chrono::system_clock::now().time_since_epoch().count());
	std::uniform_int_distribution distrib {min_rand, max_rand};

	for (int i = 0; i < 100; ++i)
		std::cout << distrib(gen) << "  ";
}

which displays 100 random integer numbers in the min_rand, max_rand range. Assuming this works, then try changing the range etc as required. When you get the expected output, then incorporate into your main program.

When you're using a 'feature' and it's not working as expected, the best thing is write a small test program, get that to work as expected, and then use in your main program.

[annolliwohe]

Ok according to what Jonnin says I just have to cancel the scientific notation in my code, so I think from what I found on the internet that I just have to add a line like this one at the beginning of my code:

std::cout.precision(5);

[annolliwohe]

It doesn't work either, I have results that don't correspond with the range requested at the beginning. I did a function outside of Sally:: to iterate the random and return the value afterwards, so it works correctly. Thanks for your help.