Generating random numbers non-repeating recursively in C





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty{ height:90px;width:728px;box-sizing:border-box;
}







0















I'm making BINGO and I need to generate each space with a random number specified below without having duplicates. So I'm going each row at a time because of letter-number combo min/max cap (B: 1-15, I: 16-30, N: 31-45, G: 46-60, O: 61-75). It seems the best way to do this is to have a recursive function. I believe that the following function is correct except for the fact that i is not increasing. How would I go about fixing this so I don't make the same or similar mistake in the future?



int main() {
srand(time(NULL));

int bMin = 1, bMax = 15;
int i = 0;
int B1[5] = {0};
initializeCard(B1, bMin, bMax, i);

// print value of B1 to make sure function correctly executed
for (i = 0; i < 5; i++) {
printf("%d ", B1[i]);
}
}

void initializeCard(int row[5], int min, int max, int i) {
row[i] = rand() % ((max + 1) - min) + min;

int temp;
for (temp = i; temp >= 0; temp--) {
if (row[i] == row[temp]) {
initializeCard(row, min, max, i);
}
}

if (i < 5) {
i++;
initializeCard(row, min, max, i);
}
}









share|improve this question




















  • 1





    Consider whether you'd do better creating an array with all the possible letter/number combinations in a fixed order that's easy to verify, and then use a Fisher-Yates shuffle on that array to generate a random permutation of the data. The fact that each entry in the array is unique ensures there are no duplicates in the shuffled data, and a good Fisher-Yates shuffle ensures each permutation is equi-probable.

    – Jonathan Leffler
    Nov 22 '18 at 6:12











  • Don't use recursion. It feels really sexy but its also a really bad idea, especially if you ever get into embedded programming where stack sizes are limited. In this case, as Osiris points out, you get runaway recursion so no matter how much memory you have and how big your stack is allowed to grow, at some point the stack will overflow.

    – AlastairG
    Nov 22 '18 at 8:48


















0















I'm making BINGO and I need to generate each space with a random number specified below without having duplicates. So I'm going each row at a time because of letter-number combo min/max cap (B: 1-15, I: 16-30, N: 31-45, G: 46-60, O: 61-75). It seems the best way to do this is to have a recursive function. I believe that the following function is correct except for the fact that i is not increasing. How would I go about fixing this so I don't make the same or similar mistake in the future?



int main() {
srand(time(NULL));

int bMin = 1, bMax = 15;
int i = 0;
int B1[5] = {0};
initializeCard(B1, bMin, bMax, i);

// print value of B1 to make sure function correctly executed
for (i = 0; i < 5; i++) {
printf("%d ", B1[i]);
}
}

void initializeCard(int row[5], int min, int max, int i) {
row[i] = rand() % ((max + 1) - min) + min;

int temp;
for (temp = i; temp >= 0; temp--) {
if (row[i] == row[temp]) {
initializeCard(row, min, max, i);
}
}

if (i < 5) {
i++;
initializeCard(row, min, max, i);
}
}









share|improve this question




















  • 1





    Consider whether you'd do better creating an array with all the possible letter/number combinations in a fixed order that's easy to verify, and then use a Fisher-Yates shuffle on that array to generate a random permutation of the data. The fact that each entry in the array is unique ensures there are no duplicates in the shuffled data, and a good Fisher-Yates shuffle ensures each permutation is equi-probable.

    – Jonathan Leffler
    Nov 22 '18 at 6:12











  • Don't use recursion. It feels really sexy but its also a really bad idea, especially if you ever get into embedded programming where stack sizes are limited. In this case, as Osiris points out, you get runaway recursion so no matter how much memory you have and how big your stack is allowed to grow, at some point the stack will overflow.

    – AlastairG
    Nov 22 '18 at 8:48














0












0








0








I'm making BINGO and I need to generate each space with a random number specified below without having duplicates. So I'm going each row at a time because of letter-number combo min/max cap (B: 1-15, I: 16-30, N: 31-45, G: 46-60, O: 61-75). It seems the best way to do this is to have a recursive function. I believe that the following function is correct except for the fact that i is not increasing. How would I go about fixing this so I don't make the same or similar mistake in the future?



int main() {
srand(time(NULL));

int bMin = 1, bMax = 15;
int i = 0;
int B1[5] = {0};
initializeCard(B1, bMin, bMax, i);

// print value of B1 to make sure function correctly executed
for (i = 0; i < 5; i++) {
printf("%d ", B1[i]);
}
}

void initializeCard(int row[5], int min, int max, int i) {
row[i] = rand() % ((max + 1) - min) + min;

int temp;
for (temp = i; temp >= 0; temp--) {
if (row[i] == row[temp]) {
initializeCard(row, min, max, i);
}
}

if (i < 5) {
i++;
initializeCard(row, min, max, i);
}
}









share|improve this question
















I'm making BINGO and I need to generate each space with a random number specified below without having duplicates. So I'm going each row at a time because of letter-number combo min/max cap (B: 1-15, I: 16-30, N: 31-45, G: 46-60, O: 61-75). It seems the best way to do this is to have a recursive function. I believe that the following function is correct except for the fact that i is not increasing. How would I go about fixing this so I don't make the same or similar mistake in the future?



int main() {
srand(time(NULL));

int bMin = 1, bMax = 15;
int i = 0;
int B1[5] = {0};
initializeCard(B1, bMin, bMax, i);

// print value of B1 to make sure function correctly executed
for (i = 0; i < 5; i++) {
printf("%d ", B1[i]);
}
}

void initializeCard(int row[5], int min, int max, int i) {
row[i] = rand() % ((max + 1) - min) + min;

int temp;
for (temp = i; temp >= 0; temp--) {
if (row[i] == row[temp]) {
initializeCard(row, min, max, i);
}
}

if (i < 5) {
i++;
initializeCard(row, min, max, i);
}
}






c






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Nov 22 '18 at 6:00









duong_dajgja

1,72611636




1,72611636










asked Nov 22 '18 at 5:54









AndyAndy

32




32








  • 1





    Consider whether you'd do better creating an array with all the possible letter/number combinations in a fixed order that's easy to verify, and then use a Fisher-Yates shuffle on that array to generate a random permutation of the data. The fact that each entry in the array is unique ensures there are no duplicates in the shuffled data, and a good Fisher-Yates shuffle ensures each permutation is equi-probable.

    – Jonathan Leffler
    Nov 22 '18 at 6:12











  • Don't use recursion. It feels really sexy but its also a really bad idea, especially if you ever get into embedded programming where stack sizes are limited. In this case, as Osiris points out, you get runaway recursion so no matter how much memory you have and how big your stack is allowed to grow, at some point the stack will overflow.

    – AlastairG
    Nov 22 '18 at 8:48














  • 1





    Consider whether you'd do better creating an array with all the possible letter/number combinations in a fixed order that's easy to verify, and then use a Fisher-Yates shuffle on that array to generate a random permutation of the data. The fact that each entry in the array is unique ensures there are no duplicates in the shuffled data, and a good Fisher-Yates shuffle ensures each permutation is equi-probable.

    – Jonathan Leffler
    Nov 22 '18 at 6:12











  • Don't use recursion. It feels really sexy but its also a really bad idea, especially if you ever get into embedded programming where stack sizes are limited. In this case, as Osiris points out, you get runaway recursion so no matter how much memory you have and how big your stack is allowed to grow, at some point the stack will overflow.

    – AlastairG
    Nov 22 '18 at 8:48








1




1





Consider whether you'd do better creating an array with all the possible letter/number combinations in a fixed order that's easy to verify, and then use a Fisher-Yates shuffle on that array to generate a random permutation of the data. The fact that each entry in the array is unique ensures there are no duplicates in the shuffled data, and a good Fisher-Yates shuffle ensures each permutation is equi-probable.

– Jonathan Leffler
Nov 22 '18 at 6:12





Consider whether you'd do better creating an array with all the possible letter/number combinations in a fixed order that's easy to verify, and then use a Fisher-Yates shuffle on that array to generate a random permutation of the data. The fact that each entry in the array is unique ensures there are no duplicates in the shuffled data, and a good Fisher-Yates shuffle ensures each permutation is equi-probable.

– Jonathan Leffler
Nov 22 '18 at 6:12













Don't use recursion. It feels really sexy but its also a really bad idea, especially if you ever get into embedded programming where stack sizes are limited. In this case, as Osiris points out, you get runaway recursion so no matter how much memory you have and how big your stack is allowed to grow, at some point the stack will overflow.

– AlastairG
Nov 22 '18 at 8:48





Don't use recursion. It feels really sexy but its also a really bad idea, especially if you ever get into embedded programming where stack sizes are limited. In this case, as Osiris points out, you get runaway recursion so no matter how much memory you have and how big your stack is allowed to grow, at some point the stack will overflow.

– AlastairG
Nov 22 '18 at 8:48












1 Answer
1






active

oldest

votes


















0














You have one big problem in your initialize function:



for (temp = i; temp >= 0; temp--)
{
if (row[i] == row[temp])
{
initializeCard(row, min, max, i);
}
}


Since you start with temp = i, row[i] == row[temp] is always true, so there is no way to break the recursion. You should start from temp = i-1:



for (temp = i-1; temp >= 0; temp--)


I also cleaned the function a little bit up:



void initializeCard(int row[5], int min, int max, int i)
{
row[i] = rand() % ((max + 1) - min) + min;

int temp;
for (temp = i-1; temp >= 0; temp--) //Do not check i itself
{
if (row[i] == row[temp])
{
initializeCard(row, min, max, i);
return; //No need to continue this function
}
}

i++; //Increment before checking, otherwise it is executed with i=5 which is out of bounds
if (i < 5)
{
initializeCard(row, min, max, i);
}
}


Of course the better approach would be to not try again if a duplicate was found, instead eliminate the possibility that you can create a duplicate. For example write all possible numbers in one array and delete the chosen one every time.






share|improve this answer


























    Your Answer






    StackExchange.ifUsing("editor", function () {
    StackExchange.using("externalEditor", function () {
    StackExchange.using("snippets", function () {
    StackExchange.snippets.init();
    });
    });
    }, "code-snippets");

    StackExchange.ready(function() {
    var channelOptions = {
    tags: "".split(" "),
    id: "1"
    };
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function() {
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled) {
    StackExchange.using("snippets", function() {
    createEditor();
    });
    }
    else {
    createEditor();
    }
    });

    function createEditor() {
    StackExchange.prepareEditor({
    heartbeatType: 'answer',
    autoActivateHeartbeat: false,
    convertImagesToLinks: true,
    noModals: true,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: 10,
    bindNavPrevention: true,
    postfix: "",
    imageUploader: {
    brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
    contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
    allowUrls: true
    },
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    });


    }
    });














    draft saved

    draft discarded


















    StackExchange.ready(
    function () {
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53424667%2fgenerating-random-numbers-non-repeating-recursively-in-c%23new-answer', 'question_page');
    }
    );

    Post as a guest















    Required, but never shown

























    1 Answer
    1






    active

    oldest

    votes








    1 Answer
    1






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes









    0














    You have one big problem in your initialize function:



    for (temp = i; temp >= 0; temp--)
    {
    if (row[i] == row[temp])
    {
    initializeCard(row, min, max, i);
    }
    }


    Since you start with temp = i, row[i] == row[temp] is always true, so there is no way to break the recursion. You should start from temp = i-1:



    for (temp = i-1; temp >= 0; temp--)


    I also cleaned the function a little bit up:



    void initializeCard(int row[5], int min, int max, int i)
    {
    row[i] = rand() % ((max + 1) - min) + min;

    int temp;
    for (temp = i-1; temp >= 0; temp--) //Do not check i itself
    {
    if (row[i] == row[temp])
    {
    initializeCard(row, min, max, i);
    return; //No need to continue this function
    }
    }

    i++; //Increment before checking, otherwise it is executed with i=5 which is out of bounds
    if (i < 5)
    {
    initializeCard(row, min, max, i);
    }
    }


    Of course the better approach would be to not try again if a duplicate was found, instead eliminate the possibility that you can create a duplicate. For example write all possible numbers in one array and delete the chosen one every time.






    share|improve this answer






























      0














      You have one big problem in your initialize function:



      for (temp = i; temp >= 0; temp--)
      {
      if (row[i] == row[temp])
      {
      initializeCard(row, min, max, i);
      }
      }


      Since you start with temp = i, row[i] == row[temp] is always true, so there is no way to break the recursion. You should start from temp = i-1:



      for (temp = i-1; temp >= 0; temp--)


      I also cleaned the function a little bit up:



      void initializeCard(int row[5], int min, int max, int i)
      {
      row[i] = rand() % ((max + 1) - min) + min;

      int temp;
      for (temp = i-1; temp >= 0; temp--) //Do not check i itself
      {
      if (row[i] == row[temp])
      {
      initializeCard(row, min, max, i);
      return; //No need to continue this function
      }
      }

      i++; //Increment before checking, otherwise it is executed with i=5 which is out of bounds
      if (i < 5)
      {
      initializeCard(row, min, max, i);
      }
      }


      Of course the better approach would be to not try again if a duplicate was found, instead eliminate the possibility that you can create a duplicate. For example write all possible numbers in one array and delete the chosen one every time.






      share|improve this answer




























        0












        0








        0







        You have one big problem in your initialize function:



        for (temp = i; temp >= 0; temp--)
        {
        if (row[i] == row[temp])
        {
        initializeCard(row, min, max, i);
        }
        }


        Since you start with temp = i, row[i] == row[temp] is always true, so there is no way to break the recursion. You should start from temp = i-1:



        for (temp = i-1; temp >= 0; temp--)


        I also cleaned the function a little bit up:



        void initializeCard(int row[5], int min, int max, int i)
        {
        row[i] = rand() % ((max + 1) - min) + min;

        int temp;
        for (temp = i-1; temp >= 0; temp--) //Do not check i itself
        {
        if (row[i] == row[temp])
        {
        initializeCard(row, min, max, i);
        return; //No need to continue this function
        }
        }

        i++; //Increment before checking, otherwise it is executed with i=5 which is out of bounds
        if (i < 5)
        {
        initializeCard(row, min, max, i);
        }
        }


        Of course the better approach would be to not try again if a duplicate was found, instead eliminate the possibility that you can create a duplicate. For example write all possible numbers in one array and delete the chosen one every time.






        share|improve this answer















        You have one big problem in your initialize function:



        for (temp = i; temp >= 0; temp--)
        {
        if (row[i] == row[temp])
        {
        initializeCard(row, min, max, i);
        }
        }


        Since you start with temp = i, row[i] == row[temp] is always true, so there is no way to break the recursion. You should start from temp = i-1:



        for (temp = i-1; temp >= 0; temp--)


        I also cleaned the function a little bit up:



        void initializeCard(int row[5], int min, int max, int i)
        {
        row[i] = rand() % ((max + 1) - min) + min;

        int temp;
        for (temp = i-1; temp >= 0; temp--) //Do not check i itself
        {
        if (row[i] == row[temp])
        {
        initializeCard(row, min, max, i);
        return; //No need to continue this function
        }
        }

        i++; //Increment before checking, otherwise it is executed with i=5 which is out of bounds
        if (i < 5)
        {
        initializeCard(row, min, max, i);
        }
        }


        Of course the better approach would be to not try again if a duplicate was found, instead eliminate the possibility that you can create a duplicate. For example write all possible numbers in one array and delete the chosen one every time.







        share|improve this answer














        share|improve this answer



        share|improve this answer








        edited Nov 22 '18 at 6:11

























        answered Nov 22 '18 at 6:03









        OsirisOsiris

        2,572416




        2,572416
































            draft saved

            draft discarded




















































            Thanks for contributing an answer to Stack Overflow!


            • Please be sure to answer the question. Provide details and share your research!

            But avoid



            • Asking for help, clarification, or responding to other answers.

            • Making statements based on opinion; back them up with references or personal experience.


            To learn more, see our tips on writing great answers.




            draft saved


            draft discarded














            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53424667%2fgenerating-random-numbers-non-repeating-recursively-in-c%23new-answer', 'question_page');
            }
            );

            Post as a guest















            Required, but never shown





















































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown

































            Required, but never shown














            Required, but never shown












            Required, but never shown







            Required, but never shown







            Popular posts from this blog

            Guess what letter conforming each word

            Port of Spain

            Run scheduled task as local user group (not BUILTIN)