Finding random numbers without duplicates

[kingseveneye]

Hi. I am relatively new at this and wrote the following code in an attempt to choose 21 UNIQUE numbers between 1 and 52 but for some reason I am still getting duplicates. Is my use of || as OR not being used properly? Thanks for your help!

#include<iostream>
#include<cstdlib>
#include<ctime>

using namespace std;


int main(){

    srand(time(0));
    int c1=1+(rand()%52);
    int c2=1+(rand()%52);
    int c3=1+(rand()%52);
    int c4=1+(rand()%52);
    int c5=1+(rand()%52);
    int c6=1+(rand()%52);
    int c7=1+(rand()%52);
    int c8=1+(rand()%52);
    int c9=1+(rand()%52);
    int c10=1+(rand()%52);
    int c11=1+(rand()%52);
    int c12=1+(rand()%52);
    int c13=1+(rand()%52);
    int c14=1+(rand()%52);
    int c15=1+(rand()%52);
    int c16=1+(rand()%52);
    int c17=1+(rand()%52);
    int c18=1+(rand()%52);
    int c19=1+(rand()%52);
    int c20=1+(rand()%52);
    int c21=1+(rand()%52);
    while(c2==c1){
        c2=1+(rand()%52);
    }
    while(c3==(c1||c2)){
        c3=1+(rand()%52);
    }
    while(c4==(c1||c2||c3)){
        c4=1+(rand()%52);
    }
    while(c5==(c1||c2||c3||c4)){
        c5=1+(rand()%52);
    }
    while(c6==(c1||c2||c3||c4||c5)){
        c6=1+(rand()%52);
    }
    while(c7==(c1||c2||c3||c4||c5||c6)){
        c7=1+(rand()%52);
    }
    while(c8==(c1||c2||c3||c4||c5||c6||c7)){
        c8=1+(rand()%52);
    }
    while(c9==(c1||c2||c3||c4||c5||c6||c7||c8)){
        c9=1+(rand()%52);
    }
    while(c10==(c1||c2||c3||c4||c5||c6||c7||c8||c9)){
        c10=1+(rand()%52);
    }
    while(c11==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10)){
        c11=1+(rand()%52);
    }
    while(c12==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10||c11)){
        c12=1+(rand()%52);
    }
    while(c13==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10||c11||c12)){
        c13=1+(rand()%52);
    }
    while(c14==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10||c11||c12||c13)){
        c14=1+(rand()%52);
    }
    while(c15==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10||c11||c12||c13||c14)){
        c15=1+(rand()%52);
    }
    while(c16==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10||c11||c12||c13||c14||c15)){
        c16=1+(rand()%52);
    }
    while(c17==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10||c11||c12||c13||c14||c15||c16)){
        c17=1+(rand()%52);
    }
    while(c18==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10||c11||c12||c13||c14||c15||c16||c17)){
        c18=1+(rand()%52);
    }
    while(c19==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10||c11||c12||c13||c14||c15||c16||c17||c18)){
        c19=1+(rand()%52);
    }
    while(c20==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10||c11||c12||c13||c14||c15||c16||c17||c18||c19)){
        c20=1+(rand()%52);
    }
    while(c21==(c1||c2||c3||c4||c5||c6||c7||c8||c9||c10||c11||c12||c13||c14||c15||c16||c17||c18||c19||c20)){
        c21=1+(rand()%52);
    }
    cout<<c1<<endl;
    cout<<c2<<endl;
    cout<<c3<<endl;
    cout<<c4<<endl;
    cout<<c5<<endl;
    cout<<c6<<endl;
    cout<<c7<<endl;
    cout<<c8<<endl;
    cout<<c9<<endl;
    cout<<c10<<endl;
    cout<<c11<<endl;
    cout<<c12<<endl;
    cout<<c13<<endl;
    cout<<c14<<endl;
    cout<<c15<<endl;
    cout<<c16<<endl;
    cout<<c17<<endl;
    cout<<c18<<endl;
    cout<<c19<<endl;
    cout<<c20<<endl;
    cout<<c21<<endl;
    }

[MatthewRock]

(c1 || c2) is wrong. You should check for* :

while((c3 == c1) || (c3 == c2))
{
//code...
}

* Actually, you should find better way of doing it(array?), because this code looks terrible, and must have felt the sime when you were writing it.

[kingseveneye]

Thanks for your help! I'm really just a beginner and don't understand arrays much yet. I'm still learning ;)

[Albatross]

Arrays would indeed be a better way of dealing with this problem. They'll really reduce your code's size and make it more intuitive to deal with.

You can either have an array of numbers that you'd shuffle and get numbers from sequentially (like a deck, which I suspect you're trying to emulate), an array of numbers that you'd pick random numbers from, copy elements over to overwrite the gotten numbers, and decrease a counter storing the "size" of the array (like a deck that you pick out cards from the middle of), or have an array of bools that indicate whether a certain number was gotten or not.

int foo[52]; //Initialize the array.
int size = 52; //Store the size of the array.
for (n=0; n<size; ++n) //Array indexes go from 0 to size-1. This is important!
  foo[n] = n;

//Set the (n+1)th element of the array to n.

http://www.cplusplus.com/doc/tutorial/arrays/

Good luck!

-Albatross

Note: Yes, the example uses C-style arrays rather than std::array. I recommend std::array for most cases over C-style arrays, but the traditional approach to C++ teaching covers class template instantiation relatively late.

[kingseveneye]

Thanks to you both. I know that my code was vastly inefficient. I'll check out that tutorial and find a better way. I'm trying to learn this stuff on my own and it's not easy lol.

[kingseveneye]

I think I've built a better formula to run this, but I'm still confused about the following line that I needed to add:

int c[21];

The program still seems to run fine no matter what number I insert inside the []. Why is that? I thought that any number under 21 would result in an error but it does not.


Here's the new code I came up with to replace all the junk from above:

#include<iostream>
#include<cstdlib>
#include<ctime>

using namespace std;

int c[21];

int main(){

srand(time(0));

for(int x=1;x<=21;x++){
c[x]=1+(rand()%52);
int y=1;
while(y!=x){
if(c[x]==c[y]){
c[x]=1+(rand()%52);
y=1;
}
y++;

}
cout<<c[x]<<endl;
}

}

[xismn]

I thought that any number under 21 would result in an error but it does not.

You mean, any number over 21, right?

Here's an integer array with 21 elements:

int a[21];

I can access these elements with an index. The first element is at index location zero.

a[0] = 128;//whatever I want to do with it
std::cout << a[0] << std::endl;

Accessing an element that's not within the bounds of the array will result in undefined behavior.

int a[21];
for(unsigned short i=0; i<22; ++i) {//undefined behaviour
	a[i] = 0;
}

At the end of that for loop, I'm attempting to access index location 21, which would be the 22nd element - which doesn't exist.
To be more specific, the memory that would belong to the 22nd element exists, but doesn't belong to the array. It belongs to something else, so by changing it, you evoke undefined behavior.

Arrays are "left over" from C, where there was no way to ensure bounds-checking. Now, C++ offers different ways of bounds-checking.
For instance, the STL vector container can access elements with or without bounds checking.