Can't shuffle cards once for each element

Question

I am trying to write a code that shuffles every element at least once and it didn't work for me.

The code I tried is :

#include <stdio.h>
#include <time.h>
#include <stdlib.h>

void show(int[],int);

void shuffle(int[],int,int*);

int main (void)
{
     int karten[]={1,2,3,4,5,6,7,8,9,10};
     int n = sizeof(karten)/sizeof(int);
     int s=0;
     srand(time(NULL));
     printf("Karten vor dem Mischen: \n");
     show(karten,n);
     shuffle(karten,n,&s);
     printf("Karten nach dem Mischen:\n");
     show(karten,n);
     return 0;
}
void show(int karten[],int n)
{
    for(int i=0;i<n;i++)
    {
        printf("%d,",karten[i]);
    }
    printf("\n");
}
void shuffle(int karten[],int n,int *s)
{
    int i=0;
    int d=0;
    int vi;
    int vd;
    int q;
    *s=0;
    int *v=(int*)malloc(sizeof(int)*n);
    q=0;
    while(1)
    {
        i=rand()%10;
        d=rand()%10;
        vi=karten[i];
        vd=karten[d];
        karten[d]=vi;
        karten[i]=vd;
        *s=*s+1; 
        v[i]=1;
        v[d]=1;
        for(int b=0;b<=n;b++)
        {
            if(v[b]==1)
            {
                q++;
            }
        }
        if(q==n)
        {
            break;
        }
    }
    printf("Es wurden %d Vertauschungen gemacht\n",*s);
    free(v);
}

The error is that the code works some times and it does not work some times. When it works, I think it doesn't work right because the shuffle times are (3) or (4). I tried to make it as simple as possible.

Your swap is rather complicated. The memory that you get from `malloc` isn't initialised. Try `calloc` for zeroed memory. — M Oehm, Jan 25 '16 at 15:18
You don't clear v to zeros, and that is where you track the Vertauschungen (exchanges or swaps). After one exchange there will two elements of v that contain '1', so your q could reach the limit 'n' early and you stop swapping. — BryanT, Jan 25 '16 at 15:18

score 2 · Answer 1 · answered Jan 25 '16 at 15:17

There are three issues.

The first is that you're not initializing v after the memory is allocated. You should either set all values to 0 or just use calloc which will do that for you.

The second is in the for loop where you're checking if all cards have been shuffled:

for(int b=0;b<=n;b++)

Your array index b is ranging from 0 to n, however because the array has n elements, the valid indexes are 0 to n-1. So change <= to <:

for(int b=0;b<n;b++)

The third is in your use of q. You should be initializing it to 0 at the start of the while loop, not before you enter it. Otherwise, you'll be adding the count of shuffled elements from the previous run to the current count.

For example, suppose on the first iteration i is 2 and d is 3. Then 2 total elements will have been swapped. You'll increment q by 2, so now q is equal to 2. Now suppose on the next iteration i is 4 and d is 5. A total of 4 elements have been swapped. So you increments q 4 times. But q has the value of 2 from the last iteration, so now q is 6. One more iteration, and q will be at least 12. So the condition q==n will never be met.

user3121023 · Answer 2 · 2016-01-25T15:44:15.987

1

Try this for your shuffle. It gets a random number swap in the range 0 to n-1 (making sure swap does not equal n-1) and then decrements n. The two values indexed by n and swap are swapped. Continue until n is zero.

void shuffle(int karten[],int n)
{
    int swap = 0;
    int temp = 0;

    while(n)
    {
        do {
            swap = rand ( ) % n;
        } while ( n > 1 && swap == n - 1);
        n--;
        temp = karten[swap];
        karten[swap] = karten[n];
        karten[n] = temp;
    }
}

edited Jan 25 '16 at 15:44

answered Jan 25 '16 at 15:17

user3121023

8,181
5
18
16

I wonder if the requirement to shuffle each card at least once disallows a card being shuffled to its original position? Allowing a card to remain in the same place after shuffling the pack would result in the shuffle being more random, as there would be one more place in the pack where each card can end up; there would be n! possible shuffles instead of (n-1)! possible shuffles. To allow that, the inner `do`/`while` loop can be replaced with just the single statement it contains. – Ian Abbott Jan 25 '16 at 17:09

Can't shuffle cards once for each element

2 Answers2