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;
}
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
add a comment |
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
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
add a comment |
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
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
c
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
add a comment |
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
add a comment |
1 Answer
1
active
oldest
votes
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.
add a comment |
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
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
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.
add a comment |
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.
add a comment |
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.
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.
edited Nov 22 '18 at 6:11
answered Nov 22 '18 at 6:03
OsirisOsiris
2,572416
2,572416
add a comment |
add a comment |
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.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
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
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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
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