0

Hi so I'm trying to make a random number generator as part of my calculator. I have been learning C++ for the past few weeks and I'm not sure what the problem is. Codeblocks can detect no errors but it will not function correctly. 

          int first;
    int last;
    int counter;

    cout << "Enter range of the numbers you want to generate. eg. Between 1 and 20.\n"  << endl;
    cout << "Between..." << endl;
    cin >> first;
    cout << "And..." << endl;
    cin >> last;
    cout << "Enter the amount of numbers you want to generate: " << endl;
    cin >> counter;
    cout << endl;

    srand(time(0));

    for (int first; last < counter; first++)
    {
        cout << 1+(rand()%last) << endl;
    }
user3421181
  • 3
  • 1
  • What does it do? What do you want it to do? Any errors? – OMGtechy Mar 14 '14 at 18:42
  • 5
    You should use a [`random_engine`](http://en.cppreference.com/w/cpp/numeric/random) – Borgleader Mar 14 '14 at 18:43
  • 8
    `for (int first; last < counter; first++)` <- wow is this wrong. You don't initialize `first`, loop condition does not include `first`, and nothing in the body changes what the loop condition checks... – crashmstr Mar 14 '14 at 18:43
  • 7
    @crashmstr: Also, shadowing – Mooing Duck Mar 14 '14 at 18:44
  • 1
    Related: [What is the best way to generate random numbers in C++?](https://stackoverflow.com/q/9471604) you should use the c++11 random facilities if you can. – AliciaBytes Mar 14 '14 at 18:49
  • 1
    possible duplicate of [What is the best way to generate random numbers in C++?](http://stackoverflow.com/questions/9471604/what-is-the-best-way-to-generate-random-numbers-in-c) – Brad Rem Mar 14 '14 at 19:15

3 Answers3

4
for (int first; last < counter; first++)
  cout << 1+(rand()%last) << endl;

From the prompt text it sounds like first and last have nothing to do with the number of values you want to generate, so they should not appear inside the for(...), only in the body.

for (int i=0; i<counter; ++i)
  cout << first+(rand()%(last-first+1)) << endl;

Also, instead of using rand() and srand() you should use the C++11 <random> library.

#include <random>

// create and seed a source of random data
std::random_device r;
std::seed_seq seed{r(), r(), r(), r(), r(), r(), r(), r()};
std::mt19937 rng(seed);

// define the distribution you want
std::uniform_int_distribution<int> dist(first, last);

for (int i = 0; i < counter; ++i) {
  std::cout << dist(rng) << '\n';
}

I think in general it's easier to use, although the syntax may be a bit mysterious for beginners. The above declares a couple of variables dist and rng, initializes them, and then uses dist like a function to produce the values you want.

Note in particular how much simpler it is to create the distribution dist than for you to compute a distribution yourself: first+(rand()%(last-first+1)). That calculation may not even produce a uniform distribution: some values may be produced more often than others. So <random> is better because it's easier to use and the meaning is clearer and more explicit.

bames53
  • 86,085
  • 15
  • 179
  • 244
3

There are a couple issues here. First, your for loop is wrong. What you really want is:

for (int i = 0; i < counter; ++i)

This will generate the requested amount of random numbers.

Secondly, to generate a random number in the range of "first" to "last", inclusive, you need to determine the difference between "last" and "first", get a random number in that range, then translate that value up to be in the range between "first" and "last":

cout << first + (rand() % (last - first + 1)) << endl

The "+1" is to guarantee that "last" is inclusive with the allowed range.

tawnos178
  • 761
  • 5
  • 3
0

I think you mean for (int first=0; first < counter; first++)

Also, taking modulus of a random number output compromises its properties. Instead you should scale it: 1.0 * rand() / RAND_MAX * last;. RAND_MAX is defined by the standard and 1.0 means stops integer division.

Peter - Reinstate Monica
  • 15,048
  • 4
  • 37
  • 62
P45 Imminent
  • 8,319
  • 4
  • 35
  • 78