Compare all elements within a single array

Question

Just want to start off apologizing if this has been answered and just should be worded differently so I was unable to find it. I've been working on this particular problem for a few hours and it's possible my Google Fu is weak atm.

For my c++ class I have to create a random number generator that collects data from the user to specify the number of digits the numbers must be and the number of rands to create. I can create the randoms just fine, the only problem is the numbers can not duplicate at any point and the if loop I created at the end to decrement returns true always and I can't figure out why. Because of this, the loop never moves on to increment i. If I'm not mistaken, it should not even be possible to have a duplicate entry on the first try right?

void TargetGen::genNumbers()
{
    int mod = 0;
    int baseMod = 0;

    if(Digits != 1)
    {
        baseMod = pow(10.0,(Digits -1));
    }
    mod = (pow(10.0,Digits))-baseMod;


    for(int i=0;i<Numbers;i++)
    {
        cout << "front of i loop, value of i: " << i << endl;
        int randomTemp;

        randomTemp = rand() % mod + baseMod;

        targets[i] = randomTemp;
        cout << "rand: " << targets[i] << endl;
        for(int k = 0;k <= Numbers; k++)
        {
            if(targets[k] == targets[i])
            {
                cout << targets[i] << endl;
                i--;
            }
            cout << " k looping, k value: " << k << endl;
            cout << " k loop, value of i: " << i << endl;

        }
    }
}

Easiest way to avoid duplicates, and do not keep all numbers in the memory -- to use sequenced numbers 1,2,..N, and encrypt each by any symmetric block cypher, for example, DES. Since number can be decrypted back, then impossible have 2 differrent inputs, whose maps to same cryptogram. — olegarch, Nov 07 '13 at 00:09
possible duplicate of [Algorithm to select a single, random combination of values?](http://stackoverflow.com/questions/2394246/algorithm-to-select-a-single-random-combination-of-values) — Jerry Coffin, Nov 07 '13 at 00:10
Make `target` a `std::set` and keep adding random numbers until you get to the desired size. — Zac Howland, Nov 07 '13 at 00:10
The loop over `k` should likely be `for (int k = 0; k < i; ++k) ...`. Otherwise, the loop will always reach `k == i`, at which point, of course, `targets[k] == targets[i]` and you are not making any progress. — Igor Tandetnik, Nov 07 '13 at 00:12
The problem statement sounds strange to me though. A random number generator that doesn't repeat itself is not really very random: if you throw a die six times, do you expect it to produce six distinct numbers very often? Also, if I ask you to generate 1000 2-digit numbers, you are going to have a difficult time. — Igor Tandetnik, Nov 07 '13 at 00:15
I actually have added logic in another function to disallow requesting more numbers than would be possible to present. But I do agree, RNG without duplication seems silly. — user2962750, Nov 07 '13 at 00:27

score 0 · Answer 1 · answered Nov 07 '13 at 00:09

0

Have you initialized "targets" anywhere? I'm a little weak in c++ (i haven't coded in it in years), but I don't see it it declared anywhere...so I'm guessing that it's always returning true because targets isn't initialized anywhere...I could be wrong though.

answered Nov 07 '13 at 00:09

deweyredman

1,440
1
9
12

If it hadn't been declared, the code would not compile. It looks like it's an attribute of the class. – José Tomás Tocino Nov 07 '13 at 00:10
ahhh that makes sense. That's what I get for coding in python for too long >__ – deweyredman Nov 07 '13 at 00:26

score 0 · Answer 2 · answered Nov 07 '13 at 00:13

0

The inner loop is comparing the new entry against all entries in the array, including itself. Limit k to be less than i and you should be fine.

answered Nov 07 '13 at 00:13

Mike Woolf

1,210
7
11

score 0 · Answer 3 · answered Nov 07 '13 at 00:15

I suggest you follow the advice from Zac Howland from the comments, use a set instead of a vector and keep checking the size, or rather whether the insertion took place. Something like:

int insertedNumbers = 0;
set<int> targets;
while (insertedNumbers != Numbers) {
    int randomTemp = rand() % mod + baseMod;

    pair<set<int>::iterator, bool> insertionResult = targets.insert(randomTemp);

    if(insertionResult.second) insertedNumbers ++;
}

cout << "Random numbers:" << targets.size() << endl; // Should equal Numbers

score 0 · Answer 4 · answered Nov 07 '13 at 00:19

I have a few suggestions for you:

1) rand() normally requires seeding. If every time you run it you get the same number, you're probably forgetting to seed: http://www.cplusplus.com/reference/cstdlib/rand/

2) If you can't repeat the random numbers you're generating, and you're not creating an incredibly large number of them, you might want to put each result into a set or a sorted list. That way you can just check against what you know you have created.

3) generally Numbers would indicate a class since the N is capitalized. Use numberOfNumbers if you must, it's a clearer variable name.

Compare all elements within a single array

4 Answers4