Become a MacRumors Supporter for $50/year with no ads, ability to filter front page stories, and private forums.

Sam Yikin

macrumors regular
Original poster
Oct 28, 2007
230
0
Alright, I'm just starting to learn C, and I've run into a problem. This code, which is adapted from a book I'm using, isn't working correctly.
#include <stdio.h>
int main ()
{
char idFound = 0;
int counter; //Used for sequential search for loop, and for getting balance from the second array, which is parallel
int enteredID; // The ID that the user entered, to be searched
int id[5] = {1,2,3,765,90}; // Used to store people's ID codes
float balance[5] = {101.23,41,50,2,89}; //Used to store their current account balance
while (idFound != 1)
{
printf ("Please enter an account number to search: ");
scanf (" %d", &enteredID);
for (counter=0; counter<=5; ++counter) // Searches for the number the user entered
{
if (enteredID == id[counter])
{
idFound=1; // Tells program that it found the proper ID
break; // Breaks the loop early if the entered ID is found
}
else continue;
}
if (idFound) // Checks to see if the ID was found, then does stuff inside if
{
if ((balance[counter]) >= 50) //If the user's balance is over fifty it displays a message saying so
printf ("The current balance, %.2f, is over or equal to fifty. You're good to go!\n", balance[counter]);
else
printf ("The current balance, %.2f, is under fifty. You have some earning to do.\n", balance[counter]);
}
else
puts ("The ID you entered was not found. Please try again.");
}
return 0;
}
The problem is that ifFound- the variable used to determine if the User's ID was found- is always setting to 1. I can't figure out why. Any help would be appreciated. Also, I'm very new to this, so excuse me if its something very obvious.
 

lee1210

macrumors 68040
Jan 10, 2005
3,182
3
Dallas, TX
There was an off-by-one error in one of your loop conditions.

Code:
for (counter=0; counter<=5; ++counter)

should be:

Code:
for (counter=0; counter<5; ++counter)
because your arrays are 5 long, so the indices are 0-4. You got undefined behavior going off the end of the array, and it just so happened that enteredID was placed in memory just after your id array, so it was being compared to itself in the loop when counter was equal to 5.

The style here is pretty bad, too, but you didn't ask about that so I won't go into it. Maybe the book has that style, so I'll give you the benefit of the doubt.

-Lee
 

Sam Yikin

macrumors regular
Original poster
Oct 28, 2007
230
0
There was an off-by-one error in one of your loop conditions.

Code:
for (counter=0; counter<=5; ++counter)

should be:

Code:
for (counter=0; counter<5; ++counter)
because your arrays are 5 long, so the indices are 0-4. You got undefined behavior going off the end of the array, and it just so happened that enteredID was placed in memory just after your id array, so it was being compared to itself in the loop when counter was equal to 5.

The style here is pretty bad, too, but you didn't ask about that so I won't go into it. Maybe the book has that style, so I'll give you the benefit of the doubt.

-Lee

Ahhhhh, thank you. And about the style, I'm just using this book to learn the concepts before I go on to more advanced material, I'm trying not to learn the style/habits from this book.
Any tips on how to improve WOULD be appreciated though.
 

lee1210

macrumors 68040
Jan 10, 2005
3,182
3
Dallas, TX
Ahhhhh, thank you. And about the style, I'm just using this book to learn the concepts before I go on to more advanced material, I'm trying not to learn the style/habits from this book.
Any tips on how to improve WOULD be appreciated though.

Your first exposure to code will have an impact on your style initially, so you should just be weary.

My main gripe is the use of curly braces. I always like to keep them on the same line as the control structure that the block belongs to. So for the main function header, I would put the brace on the same line. For a for, while, if, etc. structure it would go on the same line. Also, for any conditional I always prefer to use curly braces in case what needs to go on in the conditional needs to be expanded. If it does, you have to add braces. If it doesn't, it's likely to look bad. Especially if there is an if statement with braces, and an else without. That is just unsightly.

My other gripe with this would be the for loop where the off-by-one error occurred. It's intent is not clear. It seems like it might be better to use a while with the condition:
Code:
counter < 5 && idFound == 0

And initialize counter outside of the loop, then increment it inside the loop. There's many ways to solve this problem, but a for loop that you don't really intend to finish unless there is failure that uses a prefix increment does not seem like the best to me. Prefix increments occasionally serve a solid purpose, but generally they just stand to confuse.

Hope this is helpful, and good luck!

-Lee
 

Sam Yikin

macrumors regular
Original poster
Oct 28, 2007
230
0
Your first exposure to code will have an impact on your style initially, so you should just be wAry.

My main gripe is the use of curly braces. I always like to keep them on the same line as the control structure that the block belongs to. So for the main function header, I would put the brace on the same line. For a for, while, if, etc. structure it would go on the same line. Also, for any conditional I always prefer to use curly braces in case what needs to go on in the conditional needs to be expanded. If it does, you have to add braces. If it doesn't, it's likely to look bad. Especially if there is an if statement with braces, and an else without. That is just unsightly.

My other gripe with this would be the for loop where the off-by-one error occurred. It's intent is not clear. It seems like it might be better to use a while with the condition:
Code:
counter < 5 && idFound == 0

And initialize counter outside of the loop, then increment it inside the loop. There's many ways to solve this problem, but a for loop that you don't really intend to finish unless there is failure that uses a prefix increment does not seem like the best to me. Prefix increments occasionally serve a solid purpose, but generally they just stand to confuse.

Hope this is helpful, and good luck!

-Lee
Thank you, that helped a lot. That way of doing curly braces sounds a lot more intuitive, and thanks for the tip on the prefix.
 
Register on MacRumors! This sidebar will go away, and you'll see fewer ads.