C++ for loop help

[handoverfist]

Here is my code. When user inputs number of lines and their character, a pattern is displayed. However, my pattern has 1 extra line than the user inputs. I know it has to do with my loop parameters. Can anyone shed some light? Thanks in advance.


int main( )
{
int x;
char z;
cout << "Please enter number of lines" << endl;
cin >> x;
cout << "Please enter your character of choice" << endl;
cin >> z;

for (x = 1; x <= 20; x = x + 1)
{
for (int y = 1; y!=x ; y = y + 1)
cout << z;
cout << endl;
}

return 0;
}

 

[irishScott]

I'm still deciphering the code, so I'll edit this in a few minutes, but for the record, use "variableName++" as opposed to "variableName = variableName + 1" . They perform the same function, but the former is more widely used and is easier to read. Also, use better variable names (ie: "numLines" for "x" and "choiceChar" for "z"). I know this seems like a waste of time, and in programs this short it doesn't really matter, but the sooner you start doing it, the easier it becomes. Assuming you're doing this as part of a High School/College course, you'll probably get praise and/or a higher grade for doing so.

To answer your original question: aCynic2 beat me to it. (See belowt)

 

[aCynic2]

There is something else I noticed:

cout << "Please enter number of lines" << endl;
cin >> x;

Here you set x to the input value...

for (x = 1; x <= 20; x = x + 1)
{
for (int y = 1; y!=x ; y = y + 1)
cout << z;
cout << endl;
}

Then you overwrite it in the first loop.

 

[irishScott]

.

 

[Markbnj]

Originally posted by: aCynic2
There is something else I noticed:

cout << "Please enter number of lines" << endl;
cin >> x;

Here you set x to the input value...

for (x = 1; x <= 20; x = x + 1)
{
for (int y = 1; y!=x ; y = y + 1)
cout << z;
cout << endl;
}

Then you overwrite it in the first loop.

I don't think x is overwritten, since the for() creates a scope, but it might as well be since it is hidden. perhaps that's what you meant... overridden? I would think the compiler would warn on this.

 

[aCynic2]

Originally posted by: Markbnj
I don't think x is overwritten, since the for() creates a scope, but it might as well be since it is hidden. perhaps that's what you meant... overridden? I would think the compiler would warn on this.

No, I meant overwritten, but only because I forgot about scoping, dang it.

 

[Neverm1nd]

x gets overwritten. The for loop doesn't declare a new variable ( or it would have looked like: for(int x... ).
Another note on style: in c++, use for(x = 0; x < 20... rather than x = 1; x <= 20.

 

[aCynic2]

Consider using fuller, more descriptive variable names:

int main( )
{
int num_rows, current_row;
char outputch;
cout << "Please enter number of lines" << endl;
cin >> num_rows;
cout << "Please enter your character of choice" << endl;
cin >> z;

for (current_row = 0; current_row < 20; x++)
{
for (int y = 0; y!=x ; y++) // <--- Not sure which x this should be, perhaps the inputed x?
cout << outputch;
cout << endl;
}

return 0;
}

Note: I consider "for (int y = 0; y!=x ; y++)" inherently dangerous as both the x and y are moving targets. I'm not saying it's bad form, but it requires extreme caution or you could be chasing that bug for days.

Also, what if, for some odd reason, y starts greater than x? y!=x, so it happily increments for all eternity and you have a run away loop. It won't happen with this design, but in future work, you never know.

Better is y<x. y will be allowed to go up to, but not including, the value of x and no higher. If y is equal to or greater than x from the start, the loop won't execute at all.

Since you reused x in the two loops, I suspect you didn't intend that, but I don't know what you intended for each. One is the number of rows, the other is the current row.

 

[Markbnj]

Originally posted by: Neverm1nd
x gets overwritten. The for loop doesn't declare a new variable ( or it would have looked like: for(int x... ).
Another note on style: in c++, use for(x = 0; x < 20... rather than x = 1; x <= 20.

You're right, and aCynic2 you were right as well. I haven't done C++ in awhile, and apparently the change to the scoping rules for for() hasn't been implemented because it would break existing code.

 

[Aikouka]

Originally posted by: aCynic2
for (int y = 0; y!=x ; y++) // <--- Not sure which x this should be, perhaps the inputed x?
cout << outputch;
cout << endl;
}

return 0;
}

Note: I consider "for (int y = 0; y!=x ; y++)" inherently dangerous as both the x and y are moving targets. I'm not saying it's bad form, but it requires extreme caution or you could be chasing that bug for days.

Also, what if, for some odd reason, y starts greater than x? y!=x, so it happily increments for all eternity and you have a run away loop. It won't happen with this design, but in future work, you never know.

Now, I have no idea what he's trying to program other than what I can guess, but it looks like he's trying to do:

x
xx
xxx
xxxx

etc.

Based on this, you'd use the outer for loop's variable as the restraining value for y. Also, there'd be no need to worry about x < y if he checked the variable after it was being entered. But it sounds like the point of this exercise is to work with nested loops, not worry about validating input. I know sometimes in earlier courses in college, the assignment would say, "Expect proper input" or whatever. Using y != x isn't the best method though.

Although I'm a bit curious why he said to enter a number of lines and then uses a static amount like 20. To me, it looks like the outer loop's definition should be:

for (current_row = 0; current_row < x; current_row++)

and the inner loop's definition be:

for (int y = 0; y < current_row + 1; y++)

He was actually correct to start at 1 rather than 0. Starting at 0 would've given him 1 less line than the user inputted (if he was using the variable x rather than 20), because in the first instance, y = 0 and current_row = 0, so that would automatically kick out of the inner loop and not print the first character. To keep the 0 people happy, I just added 1 to current_row in the second for loop so it will effectively do the same thing.

 

[sao123]

I think you need a 4rth variable to accomplish this task... see below.

int main( )
{
int w,x,y;
char z;
cout << "Please enter number of lines" << endl;
cin >> w;
cout << "Please enter your character of choice" << endl;
cin >> z;

for (x = 1; x <= w; x++) \\loop - number of lines
{
for (y = 1; y <= x; y =++) \\loop - character per line
cout << z;
cout << endl;
}

return 0;
}

 

[handoverfist]

I took everyone's input, and here is the new code:

int main( )
{

int numlines, current_row, y;
char charchoice;
cout << "Please enter number of lines" << endl;
cin >> numlines;
cout << "Please enter your character of choice" << endl;
cin >> charchoice;

for (current_row = 0; current_row <= numlines; current_row++)
{
for (y = 0; y <= current_row; y++)
cout << charchoice;
cout << endl;
}

return 0;
}



However, regardles of how many lines are inputted, I get 10 lines of ouput everytime.

 

[stevf]

at work so cant take a closer look til later but it looks like your inner for loop is missing the braces so that the cout's are part of the first for loop

also - style-wise, you dont need the endl's on your prompts - if you take them off and end the line after the string the prompt will be on the same line - though if you do that it is good to add a few spaces at the end of the strings you print to the screen

 

[Crusty]

The inner loop doesn't need braces since the only line it was meant to execute was cout << charchoice;

 

[mlm]

Originally posted by: stevf
at work so cant take a closer look til later but it looks like your inner for loop is missing the braces so that the cout's are part of the first for loop

That shouldn't be a problem, if the second for-loop doesn't have braces, then only the following line will apply to the loop.

OP> You should either start current_row at 1 or change the <= to a <. That's causing it to write an extra line. Also, you don't need to declare current_row and y before the loops, you can do that within the for line.

 

[stevf]

your right on that - like i said cant look as close as I would like right now

 

[stevf]

ok - tried running your new code and it runs the correct number of lines +1 - as somebody mentioned earlier you need to start your loop indexes at 1. i did not see it run 10 lines each time regardless

 

[handoverfist]

Thanks for your help stevf.

It does work now, with the change you mentioned.

I am trying to reverse the pattern though, so that the pattern goes from right to left instead of left to right.

I tried using some formatting such as setw and right justification, but can't seem to get it. Any pointers? Thanks again.

 

[stevf]

are you familiar with the evil printf command? it is horrible to look at and figure out at first but it should be able to format it like that

also did you put the setw in the cout statement in the inner loop? you have to set that every time. you dont have the cout << left in the inner loop but with this kind of program you might as well


edit - i fiddled a little with it and you probably need to convert a line to a string or cstring first or try playing around with \t & \b to give yourself space then back up - at least that is all i can think of right now

 

[aCynic2]

Originally posted by: stevf
are you familiar with the evil printf command? it is horrible to look at and figure out at first but it should be able to format it like that

hehehe, I find it quite intuitive. Moreso than the cout, but I also have been programming C since the 1980s.

 

[stevf]

Agree there cynic - once you get used to using printf it is easy but the first few times you look at it and try to figure it out it looks horrible

 

[mlm]

Originally posted by: stevf
edit - i fiddled a little with it and you probably need to convert a line to a string or cstring first or try playing around with \t & \b to give yourself space then back up - at least that is all i can think of right now

If you wanted to be a simple as possible, you could just do another for-loop to cout << ' '. You would need a separate variable to keep track of how many spaces though.

 

[handoverfist]

Thanks for all the help people!

much appreciated

 

[aCynic2]

Here's an idea for prefixing the symbols with blanks, but it will require you set an upper limit, by which the user cannot be allowed to go over...

Say, set the upperlimit to 24 (generally the number of rows in a full screen display)...

so that means that the width of a line could be 24, if I'm reading your code right.

The first line will have one charchoice, the last will have numlines of charchoice.

So, mathematically, the number of preceding blanks is figured by...

f(x) = numlines - x

Where x is current_row.

that is the number of spaces that precede charchoice.

you can set up a single char array with the upper limit + 1. You must ALWAYS have one extra spot for terminating the char array with a '\0'. Failing to do so will result in the screen filling up with garbage as your program dumps memory to it (if the compiler doesn't catch it first).

you can use strnset (using 0 as the character) or memset to zero out the array, then use strnset (blankarray, ' ', numlines - current_row) to set the array with the appropriate numbe of blanks.

then it's just a matter of outputting that array, followed by the charchoice you choose.

Does this make sense?

You could also work on simply assembling the output line with appropriate number of blanks and charchoice in the char array and send that to cout.

That would be a good way to learn string manipulation and array usage.

If you need more detail, I'll work on it tomorrow night. I'm at work right now and can't focus.

 

[Aikouka]

Originally posted by: mlm
OP> You should either start current_row at 1 or change the <= to a <. That's causing it to write an extra line. Also, you don't need to declare current_row and y before the loops, you can do that within the for line.

As I mentioned earlier in my post (which was seemingly ignored), it has to start at 1 or you have to add 1 or else your first line will be blank. Since some people are anal about starting at 0, you can do that, use < and add 1 in the inner for loop's check or start from 1 and use <=.

Originally posted by: aCynic2
Here's an idea for prefixing the symbols with blanks, but it will require you set an upper limit, by which the user cannot be allowed to go over...

Say, set the upperlimit to 24 (generally the number of rows in a full screen display)...

Why do you need an upper-limit? I mean, you may go over what you can display (going back to my previous post, I doubt he needs much bound checking), but essentially, you just need to print num_rows - current_row amount of spaces (this assumes your current_row loop starts at 1, if not you need to use (current_row + 1)). You use that to print out the number of spaces.

So if you want 10 lines, your first inner loop will loop 10 - 1 times and print 9 spaces. Your second inner loop will loop 1 time and print a single character.

I still think you guys are making this too complicated for what it's supposed to be. Based on the problem itself, I assume it's for a basic level course and for basic courses, you should stick to basic ideas!