Help debug a for loop

Discuss Help debug a for loop in the C/C++ Help forum on Dev Articles. Help debug a for loop C/C++ Help forum discussing building and maintaining applications in C/C++. Find out why these languages are the foundation on which other languages are built.

I have two nested for loops inside a while loop. I need help fixing a problem with the functionality of it. It runs through the process once but not the full amount which should be three times.
Heres the code and I hope I commented enough. any help would be nice.

The objective is to choose six random numbers then choose which ones you would like to keep and which ones to clear. then it should go through the process again until either all dice are filled or amount == 3. The game is yahtzee if you know it then you'll understand the concept

__________________
---Official Member Of The Itsacon Fan Club--- Give a man a fish and he will eat for a day. Teach a man to fish and he will sit in a boat all day drinking beer.

I have two nested for loops inside a while loop. I need help fixing a problem with the functionality of it. It runs through the process once but not the full amount which should be three times.
Heres the code and I hope I commented enough. any help would be nice.

The objective is to choose six random numbers then choose which ones you would like to keep and which ones to clear. then it should go through the process again until either all dice are filled or amount == 3. The game is yahtzee if you know it then you'll understand the concept

Can't refuse help to one of the fanclub, now can I?

But firstly: Bloodlustshaman, putting while's in for's, or for's in while's, or while's in while's or for's in for's is all allowed. It doesn't make your code readable, and eats up memory like hell, but you can nest 'em infinitely if you want.

On to the code:
Seems your primary problem is the fact you don't reset the i and j counters, but there are some other small things I'd like to point out, so I'm gonna do my normal walkthrough, line by line. (Nothing like a good dissection over breakfast )

cpp Code:

Original
- cpp Code

int d = 5;//number of dice used in for loops

int dice[d];//variable used to store the initial dice roll

int pick[d];//pointer to the chosen dice

This is dangerous. It is correct that in C++ you can use variables as dimensions for arrays, but it makes your code overly slow, as the compiler has to convert this to run-time malloc calls. Which isn't necesary, since the number of dice is fixed. Better do this with a constant.

cpp Code:

Original
- cpp Code

int i = 0,j = 0,amount = 1;//used to run the for, and while loops

I've noticed before you use two types of progress counters, 0 up-to the number, and 1 up-to-and-including the number
Best to pick one, and stick with it, cause this gets you confused. Proof is in this code, where you start at 0 for the dice, and then go up-to-and-including, causing you to throw 6 dice (which gets you disqualified )
My personal preference is the 0 up-to variant, since this is also the way arrays are indexed, meaning you don't have to convert.

cpp Code:

Original
- cpp Code

for( i ; i <= d; i++ )

/*-and-*/

for( j , i; j <= d; j++ , i++ )

This is where the whole thing probably falls apart, when it comes to the second for-loop, i is alread d+1, since that was the moment the first for-loop terminated. Next time through the while-loop, this also applies to j. This causes your arrays to be incorrectly indexed, which can lead to anything from weird numbers on your screen, to a simple straightforward BSOD.

cpp Code:

Original
- cpp Code

pick[i] = dice[j];

d--;

/*-and-*/

pick[i] = dice[j] = 0;

Another problem. The way you implemented this, it is possible to overwrite earlier picked dice. What if I pick the first die both times? It'll only store the last one.

Finally, another rule-wise problem, aside from the afore-mentioned 6 dice, IIRC, you're allowed to 'unpick' dice after your second throw (throw them again for the last throw), this is not possible in your code

I took the liberty of rewriting the whole bunch (breakfast can be boring sometimes), have a look at this:

cpp Code:

Original
- cpp Code

#include <cstdlib>

#include <iostream>

#include <ctime>

#define NUM_DICE 5

#define NUM_TURNS 2

// NOTE that NUM_TURNS is only 2! You only get to pick dice twice, the final roll is final!

usingnamespace std;

int main()

{

int dice[NUM_DICE][2]; // 2-column array, one for the value, one to set if it's picked or not

int i, turn, choose; // no init values necesary

srand(time(NULL)); // randomize randomizer :-)

for(i = 0; i < NUM_DICE; i++)

dice[i][1] = -1; // pre-set all dice to 'not picked'

for(turn = 0; turn < NUM_TURNS; turn++)// using a for loop here, since we need an increment each time anyway

{// also note the use of 0-up-to logic, and the constant defined above.

cout << "Roll nr. " << (turn + 1) << endl; // fancy output :-)

for(i = 0; i < NUM_DICE; i++)

{

if(dice[i][1] < 0)

dice[i][0] = (rand() % 6) + 1;

cout << dice[i][0] << endl;

}

for(i = 0; i < NUM_DICE; i++)

{

cout << "-----------\n";

cout << dice[i][0] << endl;

cout << "-----------\n";

cout << "1] to keep \n2] to clear \n";

cin>>choose;

if( choose == 1)

dice[i][1] = 1;

else

dice[i][1] = -1;

}

}

cout << "Final result:" << endl;

for(i = 0; i < NUM_DICE; i++)

cout << dice[i][0] << endl;

system("PAUSE");

return0;

}

Hope this helps

Comments on this post

Geo.Garnett
agrees: Your the man.

BloodlustShaman
agrees: Nice stuff and thanks for telling me the facts but for the # define I have never seen them and if
you explain them that would also help a lot

__________________This is my code. Is it not nifty?

"The biggest problem encountered while trying to design a system that was completely foolproof, was, that people tended to underestimate the ingenuity of complete fools."
---Douglas Adams

Well Itsacon all's I can say is I think my problem is that I was trying so hard to do it a certain way that it screwed me up in actually doing it the best possible way. Im not to good with using # define I know what they do but I dont really understand the concept of how you use them correctly yet. If you can explain it in laymens terms for me I would also appreciate that too. Nice work by the way, but I hate to just copy and paste it to my program though . When I started I never thought that a simple game like yahtzee would be that difficult to program. I've been getting frustrated lately because it seems like I have to weed through all the bs in some of the books im reading to get to the important stuff.

Thank you for the help and as always it is much appreciated, I should be paying you to go to school, lol ...

Thanks for attempting BLS any help is always appreciated buddy.

Quote:

Im not to good with using # define I know what they do but I dont really understand the concept of how you use them correctly yet.

Ok, Looking over your code I think I understand it a little better. Is using the #define almost like making a constant variable?

Last edited by Geo.Garnett : December 30th, 2005 at 02:30 PM.
Reason: final thought

#define is a so-called pre-processor statement.
(Note: If you see experienced hackers talk about 'the CPP', they mean the C Pre-Processor, not C++)

The pre-processor is the first stage a program goes through during compilation, it includes include files, removes comments and whitespace from your code, and handles #define's.

A #define is nothing but a way to tell a compiler to replace all instances of A in the code with B.

So where I say

Code:

#define NUM_DICE 5

I'm telling the Pre-Processor that everytime he sees 'NUM_DICE' in my code, he can replace it with '5'.
Of course, I could just type '5' in my code, but then, when I have to modify the game to use 4 dice, I have to edit all those places, not to mention the trouble if I make a typo somewhere, and suddenly at one place, it uses 6 dice instead of 5 (see my comment about bad array indexing in the earlier post).

It is also possible to use #define's in a more complex way, called macros.
Take this (very commonly used) example:

Code:

#define max(a,b) (((a)>(b))?(a):(b))

Here I define a macro, that returns the maximum value of 2 'things'. I say things, cause it'll work with every variable type, struct or object that has the greater-than operator ( > ) defined (especially useful in C++).

Again, what it does, is replace every instance of max(something1, something2) with (((something1)>(something2))?(something1) : (something2)), which is a simple inline function to return the maximum value of 2 somethings.
Here the #define recognises the function-like buildup of the first parameter (with the parentheses () ) and takes the elements in between as arguments for its second part.

Code:

Homework

Rewrite the macro above to return the MINIMUM value of two somethings.
Call it min(a,b)

Of course, macros should only be used in very specific cases, usually you're better off with either typing the function out, or by using a real function.

2 last warnings when using #define's (and macros):

Just like #include, do NOT terminate the line with a semicolon ( ; ) all lines starting with a hash (#) are terminated with a line break, if you want to stretch them over multiple lines, put an escape character (backslash, \ ) at the end of the line, this will escape the linebreak, and the #define will continue on the next line.

If you make a macro, ALWAYS put every argument in the second part between INDIVIDUAL parentheses (like the (a) and (b) in my max(a,b) example). This is to make sure everything goes right if you enter an expression as either a of b.

All this may be a bit much all at once, but just practice a bit with it, and look at other peoples code, it's all pretty straightforward in the end. I know of people who've written complete programs in the CPP

Quote:

Originally Posted by Geo.Garnett

So basically what you are saying is that this is bad practice or is it ok if its done right?

It is not optimal, but not really bad either. Like I said, your program may suffer a slight performance loss, but a good optimizer might even spot this, and replace it by a constant allocation anyway.
However, you know I'm a sticker for ANSI-C in most cases, and this is not valid there, so I don't use it.
Should you do it right (which you did by the way) this wouldn't present any problems though. G++ didn't even give me a warning over it (and I always compile with -Wall, another good coding practice ), so don't worry.

Well I did my home work but im off on the median number, I had trouble for some reason to get it to calculate correctly everytime. My brain is running to fast to work right but here is what I got. and by the way man, I appreciate you taking the time with me, and walking me through everything like you do every time.

What's going wrong it the fact you only have one argument with every macro.

If you want a function (or macro) to give you the maximum of two values, you need to give 'em both values. Same goes for the median. You only specify the target variable, and neither of the others (for which it has to calculate the maximum). On the whole, specifying a target in a macro is not practical, it minimises its uses, and makes it less readable.

Also, the 'name' of the variables in the macro is of no consequence, and doesn't have to have any bearing on the names in the program. So if you define a macro max(a,b), that doesn't mean you can only use it on variables called a and b. if you use it like max(firstvariable, arrayindex[i]), it'll still work fine, just like a function.

So the corrected version of your homework would be:

cpp Code:

Original
- cpp Code

#include <cstdlib>

#include <iostream>

#define min(a,b) (((a)<(b))?(a):(b))

#define max(b,b) (((b)>(a))?(b):(a))

#define median(a,b) (((a)+(b)) / 2)

usingnamespace std;

int main()

{

int a;

int b;

cout << "Enter first number\n";

cin >> a;

cout << "Enter a second number\n";

cin >> b;

cout << "-------------------\n";

cout << "Max entered is: " << max(a,b) << endl;

cout << "Minimum entered is: " << min(a,b) << endl;

cout << "Median number is: " << median(a,b) << endl;

system("PAUSE");

returnEXIT_SUCCESS;

}

Code:

Homework

Write a macro that takes two arguments.
These represent the straight sides of a triangle.
Have the macro calculate the length of the 3rd side.
This can be done using Pythagoras (a^2 + b^2 = c^2).
You can use the math.h library for square roots.

I have failed boss. I tried to do this project but for some reason my math.h is not calculating to the second power for some reason, or Im just using it incorrectly but heres is what little I got. I also got carried away with looking through include files because now that I understand a little about define and ifndef and stuff I can kinda read the includes and make sense of them a little.

I know this isn't correct but I thought I would show you what I got so you can least see that I attempted. I know c should also be squared but I couldn't even get my compiler to do a single square root right let alone a formula more complex. For some reason it only adds the two variables together instead of doing the number squared???

By the way have a good new year and be safe tonight and watch out for da coppers too, I already got a 200$$ ticket today, and almost got another because I went through three traffic stops today in the matter of about four hours.

That was what I was doing wrong, I havent used the math library very often so its fairly new to me. This is definetly a function I am goint to start exploring with. Could have just did the a * a deal and that would have worked but like I said earlier I get so caught up trying to do something one way that I forget about the obvious. Thanks for all your help Itsacon..

Assignment 1.
Rewrite the following code so it uses defines at places where you think it would be wise. And when I say rewrite, I mean rewrite, as long as the output is the same, I don't care what you do with it.

cpp Code:

Original
- cpp Code

#include <stdio.h>

int main()

{

int i, list[100];

list[0] = 0;

for(i = 1; i < 100; i++)

list[i] = list[i - 1] + i;

for(i = 99; i > -1; i--)

printf("%d\n", list[i]);

return0;

}

PM me the result, so you lot can't cheat by looking at each other's answers :-)
When I have them from both of you, I'll post them, with comments (if necessary).

Write a macro that calculates the rms value of 3 variables.
The rms value (root-mean-square) is calculated as follows: take the square power of each value, calculate the average of the squares, then take the square root of the average. Look here for more info.
The macro must replace only the RIGHT-HAND part of the calculation, so in the code, it should be used as:

Bloodlust, assignment 1 was about the use of defines, what you did was a macro, see this post for the difference (mostly, a define has no arguments).
I'll still look at your answer, and see if it works, but you might want to enter another solution as well. It's good practice :-)

As for the fan-club, of course you don't have to be a member. But you can... :-)

Bloodlust, assignment 1 was about the use of defines, what you did was a macro, see this post for the difference (mostly, a define has no arguments).
I'll still look at your answer, and see if it works, but you might want to enter another solution as well. It's good practice :-)

As for the fan-club, of course you don't have to be a member. But you can... :-)

Alright first is first sorry about bringing this topic up but this as not been finished and I just don't drop projects just cause I can't do them.

Also I haven't been active in the furoms cause of Finals and a lot of tests in my classes. But I am back.

So let me get this straight what you wanted us to do was set it to a integer.

So for example I could have used the define function to make it equal to 100 then. In the int main()
I place that instead of the 100

#define DEFINE_NUM 100
list[DEFINE_NUM]

like that? is that what u wanted us to do just define a it to a integer and place it and play with the function?

Is that what I needed to do?
Cause I was just wondering.
Also srry about bringing this topic up after it hasnt been touched.

Alright. I did better than I thought, I thought I was going to flunk them all. But what was making my not be active were those test after cause all year we hardly had test. But after finals I had over 15 test. And for me that is A LOT!!

But now I am back.

Okay now I'll give assignment 2 a shot
dont expect that today nor tomorow cause I was in the internet for 3 HOURS trying to log in this place.[a long story].