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

int rnd(int range);
void seedrnd(void);

void main()
{
	int r;
	int j;
	char suite;
	char y;
	char face;

	seedrnd();
	r=rnd(5);

	if(r==4)
	{
		suite=0x03;
	}
		if(r==3)
	{
		suite=0x04;
	}
	if(r==2)
	{
		suite=0x05;
	}
	if(r==1)
	{
		suite=0x06;
	}

	seedrnd();
	j=rnd(14);

	if(j==1)
	{
		face=''2'';
	}
	if(j==2)
	{
		face=''3'';
	}
	if(j==3)
	{
		face=''4'';
	}
	if(j==4)
	{
		face=''5'';
	}
	if(j==5)
	{
		face=''6'';
	}
	if(j==6)
	{
		face=''7'';
	}
	if(j==7)
	{
		face=''8'';
	}
	if(j==8)
	{
		face=''9'';
	}
	if(j==9)
	{
		face=''10'';
	}
	if(j==10)
	{
		face=''J'';
	}
	if(j==11)
	{
		face=''Q'';
	}
	if(j==12)
	{
		face=''K'';
	}
	if(j==13)
	{
		face=''A'';
	}

	printf("%c %c\n",suite,face);
}

int rnd(int range)
{
	int r;

	r=rand()%range;
	return(r);
}

void seedrnd(void)
{
	srand((unsigned)time(NULL));
}
    

This source code is supposed to make a random card combo. Like Ace of Spades and what not. Occasionly though, it spits out a character I never even specified. Can anybody tell me how I can stop this? A way to make the source code more error proof? Any help, any help at all, is gretaly appreciated! ............ Guardian Angel Interactive

quote: Original post by Tiso
This source code is supposed to make a random card combo. Like Ace of Spades and what not. Occasionly though, it spits out a character I never even specified. Can anybody tell me how I can stop this? A way to make the source code more error proof? Any help, any help at all, is gretaly appreciated!

Your problem is in the use/misunderstanding of rand() and the modulus % operator.

You are expecting that rand() % NUM will give you a number from one to NUM-1, but actually it will give you a number from zero to NUM-1.

For those times your rnd() function returns zero, none of your conditional statements are taken, and the variables suite and/or face will be uninitialized. This means they will be zero at best; at worst and more likely, they will have garbage values (ie, they can have anything).

Here I include a different version of your function with the random and modulus situation corrected, and also shows you how to make things a little clearer to read by using a single switch statement instead of several if statements.

Also note that, unless you have specific reasons for calling it more than once, you only need to call the seed function once at the start. (I recommend you only call it once; calling it multiple times in the wrong place can actually reduce the randomness of your values.)

Another note: assigning '10' to a char variable is most likely NOT going to give you what you expect. A char can store a single character; '10' is two characters. If you need more than one character, you're going to have to store it that way, or use a string. Or, do what I did in the sample code here and use a 'T' for ten.

        #include <stdio.h>  #include <stdlib.h>  #include <time.h>  #include <conio.h>  #include <dos.h>    int rnd(int range);void seedrnd(void);  void main(){   int r;   int j;   char suit;   char y;   char face;     seedrnd();     r = rnd(4);   switch (r)   {      case 0:   suit = 0x03;   break;      case 1:   suit = 0x04;   break;      case 2:   suit = 0x05;   break;      case 3:   suit = 0x06;   break;   }     j = rnd(13);   switch (j)   {      case  0:   face = '2';   break;      case  1:   face = '3';   break;      case  2:   face = '4';   break;      case  3:   face = '5';   break;      case  4:   face = '6';   break;      case  5:   face = '7';   break;      case  6:   face = '8';   break;      case  7:   face = '9';   break;      case  8:   face = 'T';   break;      case  9:   face = 'J';   break;      case 10:   face = 'Q';   break;      case 11:   face = 'K';   break;      case 12:   face = 'A';   break;   }     printf("%c %c\n", suit, face);}  int rnd(int range){   int r;   r = rand() % range;   return (r);}  void seedrnd(void){   srand((unsigned)time(NULL));}        

---- --- -- -
Blue programmer needs food badly. Blue programmer is about to die!

Edited by - mossmoss on June 28, 2000 12:31:17 PM

Also, you can set up the characters in a string and use the random number as a position into it to obtain the character.

Does the same job, just a bit neater

Thank you mossmoss! The source code works quite efficiently!

Although, rarely the bug (wrong character) still shows up. Very rarely though. I''m thinking about making a while loop to stop that. I think that would work. Any ideas are still appreciated!

............
Guardian Angel Interactive

I would advise against a while loop (I assume you meant to loop until you get a valid value.) We''re programmers. The computer does exactly what we tell it; we shouldn''t have to be guessing what the output might be.

Have you attempted to debug your program? Most compiler environments include a debugger that has plenty of ways to help determine why it is still having problems. It will let you check values of your variables as your code is running.

Another thing to do, and it''s my fault for not providing it in my sample code. switch statements should always have a default case, like this:

       r = rnd(4);   switch (r)   {      case 0:   suit = 0x03;   break;      case 1:   suit = 0x04;   break;      case 2:   suit = 0x05;   break;      case 3:   suit = 0x06;   break;      default:        printf("ERROR: invalid r == %d\n", r);        exit(1);        break;   }         

I modified part of my previous example to illustrate. What happens is that if r is none of the cases specified, then it executes the default code. (You should be able to implement a similar default case for the other switch statement, with the appropriate error message.) Now, at the minimum, if r is coming back with a bad value, your program will tell you.


---- --- -- -
Blue programmer needs food badly. Blue programmer is about to die!

quote: Original post by Nivelo
Also, you can set up the characters in a string and use the random number as a position into it to obtain the character.
Does the same job, just a bit neater

Definitely right, and I should have thought of that. Tiso, what he means is something like this:

    char suits[ 4] = { 0x03, 0x04, 0x05, 0x06 };char faces[13] = { ''2'', ''3'', ''4'', ''5'', ''6'', ''7'', ''8'', ''9'', ''T'', ''J'', ''Q'', ''K'', ''A'' };  int r;int j;int suit;int face;  r = rnd(4);j = rnd(13);  suit = suits[r];face = faces[j];    

(Of course, you need to fit this into your main func.) Lot less code than if-statements or switch-statements.


---- --- -- -
Blue programmer needs food badly. Blue programmer is about to die!

Sorry if I haven''t read all the code, but may suggest a different, and I believe a better strategy.

Initialize your deck in order, like they are when you buy them. Then call a function shuffle(deck) which shuffles the cards. The cards can then be dealt out.

    void shuffle (deck){    int i;    int c1, c2;    for (i = 0; i < 1000; i++) {        // pick a card        c1 = rand () % 52;        c2 = rand () % 52;        swap (deck[c1], deck[c2]);    }}    

This is somewhat on your subject.......
Are you only going to be wanting to use one card ? I dont see how just one card in any kind of game is any good.......if you are looking to randomly place cards in a deck of cards then a method of calling this 52 times is flawed because there is a chance, a very very very good chance that you will end up with multiples of the same card.......actually it would be better to say that you would have less than a 1 in million chance of getting 52 different cards
to do that here is what you need to do......
create a linked list with 52 nodes. In each node store the suite and value of the card. Then .........

    int suite[52];int value[52];for (int i =0; i < 52; i++){   temp = headNode;   int x = rand()%52;// iterate throught the list x times and if you get to end of list then loop backaround to front of list   for (int j = 0; j < x; j++)   {      temp = temp->link;      if (link == NULL)         temp = headNode;   }   // you should now be at the xth card left in the list   suite<i> = temp->suite;   value[i] = temp->value;   RemoveNodeAtThisPlaceInList(x);}    

this is assuming you want to randomly "shuffle" your deck of cards

}

"Now go away or I shall taunt you a second time"
- Monty Python and the Holy Grail
themGames Productions

I give up on my idea, where do I insert that peice of code ncsu121978? Will it work with the code shown above? I''m assuming that code works, so I really want to use it!

Thanks!

............
Guardian Angel Interactive

Tiso, I believe what you want to do is in fact very simple. If you would restate entirely what you are truing to solve, I think I can give you a very simple and elegant solution.