C help...

Discussion in 'Mac Programming' started by Sam Yikin, Jul 19, 2008.

  1. Sam Yikin macrumors regular

    Joined:
    Oct 28, 2007
    #1
    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.
     
  2. lee1210 macrumors 68040

    lee1210

    Joined:
    Jan 10, 2005
    Location:
    Dallas, TX
    #2
    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
     
  3. Sam Yikin thread starter macrumors regular

    Joined:
    Oct 28, 2007
    #3
    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.
     
  4. lee1210 macrumors 68040

    lee1210

    Joined:
    Jan 10, 2005
    Location:
    Dallas, TX
    #4
    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
     
  5. Sam Yikin thread starter macrumors regular

    Joined:
    Oct 28, 2007
    #5
    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.
     

Share This Page