Randomizing numbers in C and storing them in an integer array

Question

I am trying to write a code which randomizes numbers between 1 and 14 (symbolizing one suit of a deck of cards). The code should store the values in an array with a limit of 52. Each number can only be stored 4 times (as there are 4 suits in a deck of cards). So, in the end, I should be displaying two randomized decks for person_a and person_b.

My problem is that the randomized decks for person_a and person_b are the same. I have no idea why. I tried seeding using srand(), and using rand() for the random number. Can someone help?

Also, I know my code is really messy and terrible. I'm sorry - this is my first time taking a C course. Below is the code:

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

#define MAX_DECK 52
#define REPETITIONS 4
#define CARDS_HIGH 14
#define CARDS_LOW 1

int
randomize_check(int value_check, int limit, int cards[])
{
    int count = 0;
    int i = 0;
    for(i=0; i<limit; i++)
    {
        if(cards[i]==value_check)
        {
            count++;
        }
    }
    if(count>REPETITIONS)
    {
        return -1;
    }
    else if (count<=REPETITIONS)
    {
        return 1;
    }
}

int
get_random(void)
{
    int random_number = 0;
    random_number = (rand()%(CARDS_HIGH-CARDS_LOW))+CARDS_LOW;

    return(random_number);
}

int * randomize_deck(void)
{

    static int cards[MAX_DECK];
    int i = 0;
    int randomize = 0;
    int check = 0;

    for (i=0; i<MAX_DECK; i++)
    {
        randomize = get_random();
        cards[i] = randomize;
        check = randomize_check(cards[i], MAX_DECK, cards);
        while((check) == -1)
        {
            randomize = get_random();
            cards[i] = randomize;
            check = randomize_check(cards[i], MAX_DECK, cards);
        }

    }
    return(cards);
}

int
main(void)
{
    srand ( time(NULL) );
    int i = 0, j = 0;

    int *person_a = randomize_deck();
    int *person_b = randomize_deck();

    for (i = 0; i < MAX_DECK; i++) //print_a
    {
        printf( "Cards[a%d]: %d\n", i, *(person_a + i));
    }

    printf("\n");

    for (j = 0; j < MAX_DECK; j++) //print_b
    {
        printf( "Cards[b%d]: %d\n", j, *(person_b + j));
    }

    return(0);
}

Try seeding it with the clock `std::srand(std::time(0));`. If you seed with the same number, you get the same sequence. — kjpus, Apr 02 '17 at 00:26
@Siguza I was hoping someone in that sub-forum would be able to help as well. — Simmon Thind, Apr 02 '17 at 00:26
Sticking with the C++ motif, `malloc` or pass the array into `randomize_deck`. C++ the answer is very different. — user4581301, Apr 02 '17 at 00:27
Agree, C or C++ matters. If you want a C solution, I will post an answer in a few minutes. — ad absurdum, Apr 02 '17 at 00:27
Pass the deck in. Try very hard to avoid that strage urge to use 'static' on things. Apart from anything else, it renders your code thread-unsafe. That might not matter here, but 'static' is a really bad habit:) — ThingyWotsit, Apr 02 '17 at 00:30
I will edit my post and take out the C++ tag, sorry. If you could help me with providing an answer in C, I would really appreciate that! — Simmon Thind, Apr 02 '17 at 00:30
If you want an alternative to checking for repeats, consider taking advantage of [the Fisher Yates Shuffle](http://stackoverflow.com/questions/3343797/is-this-c-implementation-of-fisher-yates-shuffle-correct). — user4581301, Apr 02 '17 at 00:34
@ThingyWotsit I don't know what you mean by "Pass the deck in." I'll try to avoid using static in the future, but if I remove the static initializer, the numbers I get are all weird and out of range. — Simmon Thind, Apr 02 '17 at 00:35

ad absurdum · Accepted Answer · 2017-04-02T00:50:11.777

Your problem stems from the fact that cards is declared as a static array in the randomize_deck() function. So, the first call to randomize_deck() fills this array with random cards, returning a pointer to cards. Then the second call to randomize_deck() fills the same static array with new random cards, and returns a pointer to the same static array. After all of this, both person_a and person_b point to the same static array.

One solution is to change the randomize_deck() function to accept an array argument, with a return type of void. Also, it would be best to pass the size of the array to randomize_deck(), instead of relying on a global constant. And note that in the code below I have changed the array index variables to type size_t, which is an unsigned integer type guaranteed to hold any array index, and the correct type for array indices.

void randomize_deck(int cards[], size_t deck_sz)
{

    size_t i = 0;
    int randomize = 0;
    int check = 0;

    for (i = 0; i < deck_sz; i++)
    {
        randomize = get_random();
        cards[i] = randomize;
        check = randomize_check(cards[i], deck_sz, cards);
        while((check) == -1)
        {
            randomize = get_random();
            cards[i] = randomize;
            check = randomize_check(cards[i], deck_sz, cards);
        }

    }
}

Then in main(), you declare two int arrays, one for each player, and pass these to the randomize_deck() function:

int main(void)
{
    srand ( time(NULL) );
    size_t i = 0, j = 0;

    int person_a[MAX_DECK];
    int person_b[MAX_DECK];

    randomize_deck(person_a, MAX_DECK);
    randomize_deck(person_b, MAX_DECK);

    /* ... */

    return 0;
}

Thank you so much for this answer. As I was doing this problem, I didn't think about declaring the arrays in the main function and passing them through randomize_deck(). Your explanation of the solution and why my code was not working was especially helpful! I didn't know that my code was pointing to the same array pointer, despite having randomized the cards within the array. Just one question, is there any benefit to using size_t, rather than a global variable like I did? — Simmon Thind, Apr 02 '17 at 00:54
For array indices, `size_t` is the correct type; it is possible that an `int` may not hold all array indices (however unlikely, and this point is a bit pedantic). As for using the global constant `MAX_DECK`, that is not wrong, but many consider it bad style. 1) Use of globals is generally frowned upon; you should only use globals when you have a very good reason. 2) By passing the size of the array in the function call, code seems clearer. — ad absurdum, Apr 02 '17 at 01:00

score 2 · Answer 2 · edited May 23 '17 at 12:17

"...randomized decks for person_a and person_b are the same. I have no idea why."

static int cards[MAX_DECK];

This happens because you declared your integer array as static

This means that every time the function randomize_deck is called the same int array will be operated upon and will be the same one returned.**

int * randomize_deck(void) {
    int* cards = malloc(sizeof int) * MAX_DECK); // Instantiate a new and different deck of cards every time the function is called.

    int i = 0;
    int randomize = 0;
    int check = 0;

    /* next steps randomize_deck function */

    return cards; // Now you will return a different deck of cards every time you invoke the function

}

There is one important step to take care of in your main() function. You need no deallocate the memory of the cards you allocated in randomize_deck(). So, you have to free person_a and person_b.

int main(void)
{
    srand ( time(NULL) );
    int i = 0, j = 0;

    int *person_a = randomize_deck();
    int *person_b = randomize_deck();

    /* ... */
    free(person_a); // You need to free these elements you returned,
    free(person_b);
    return 0;
}

Better :) +1 for showing the other option. BTW, it looks like you have a missing parenth. — ad absurdum, Apr 02 '17 at 01:10
Thanks man, you beat me to it, so credit (upvote) to you too :) — Santiago Varela, Apr 02 '17 at 01:11

Randomizing numbers in C and storing them in an integer array

2 Answers2