Reputation: 15
I got back my school assignment where we were asked to create some functions for the Yahtzee game, one of them were the Three of a Kind function. This is the code I wrote and the comment I got was: "You have to be able to check all numbers up to nrOfDieValues." I'm not quite sure how to check up to x numbers.. I tried using two for loops but that did not end up too well as the saving to the array got messed up. Help would be appreciated ^_^
int isThreeOfAKind(const int dieValues[], int nrOfDieValues)
{
int threeOAK, i, x = 0;
int tOAK[nrOfDievalues];
for(i = 0; i < nrOfDieValues; i++)
{
switch (dieValues[i]
{
case 1:
tOAK[0]++;
break;
case 2:
tOAK[1]++;
break;
....
....
case 6:
tOAK[5]++;
break;
}
}
for (i = 0; i < nrOfDieValues; i++)
{
if (tOAK[i] >= 3)
{
x = i+1;
threeOAK = 1;
}
}
if(threeOAK)
{
return x;
}
else
{
return 0;
}
}
Upvotes: 0
Views: 1537
Reputation: 4409
I think your problem is that the dice inputs are 1-6 while the arrays are 0-5, and that even after my comment you are still having the same problem with your loops.
Assuming that the values on the dice are 1
indexed (unlike in Unwind's answer), I've simplified the algorithm in your function. Comments are embedded in the code:
#define MaxScore 6 //maximum score on one die is 6
int isThreeOfAKind(const int dieValues[], int nrOfDieValues)
{
int i;
int tOAK[MaxScore];
memset(tOAK, 0, MaxScore); //initialise to 0
for(i = 0; i < nrOfDieValues; i++) //go through each of the inputs
{
tOAK[dieValues[i]-1]++; //increment the count of the result
if (tOAK[dieValues[i]-1]==3)
{ //if we have now seen 3 results
return dieValues[i]; //of that value, return it. we don't
} //need to process the rest.
}
return 0; //if we haven't found a three of a kind, return 0.
}
Edit: Ahah! Can't believe I missed this:
int tOAK[nrOfDievalues];
This line is a likely culprit. You are creating an array of the same size as your input. This will work fine for an input such as: {1,2,3,4,5,6,1,2,3}
but will break on, for example: {6,5,4}
.
What this array should be doing is counting the number of times each result appears, which means that you need the same number of cells as possible scores from one input. So in your case, 6
.
What your code was doing however was creating an array of the same size of the input, so when for example, you called isThreeOfAKind({6},1)
as input, you created an array tOAK[1]
.
Trying to reference tOAK[6]
leads to out of bounds access leading to a crash.
I've fixed this in the code above.
Note, you could change MaxScore
to make the function work for other games too: e.g. for a hand of cards, set MaxScore
to 13
(aces low) or 14
(aces high) and it will find a Three of a Kind in that hand too.
Upvotes: 1
Reputation: 9570
Another issue I can see in your code: seems to me you confuse the number of die values with the number of dice - I don't know how many dice are thrown in the game, so what are possible values of nrOfDieValues
, but tOAK
keeps counters for every die value, so it should be declared tOAK[6]
(for ordinary cubic dice).
By the way, did you remember to initialize the counters to zeros before you start counting...?
Upvotes: 0
Reputation: 399843
Remove the switch()
, all it does it map a value of dieValues[]
to an index into tOAK[]
, and that mapping is pretty basic:
for(i = 0; i < nrOfDieValues; i++)
{
tOAK[dieValues[i]] += 1;
}
This works the same, but assumes all indexes are 0-based.
Upvotes: 1