Loop problem, HELP please!

Discussion in 'C++' started by Francis Bell, May 25, 2004.

  1. Francis Bell

    Francis Bell Guest

    Hello, I've got a 25 line file with lines of data like this:
    sp/spinnerbait/AAA Lures/Mad Phil/silver/bass/1/1
    The first field is the code that determines what to do. I need to loop
    through this data file and, based on that first field, execute different
    cases in a switch statement (yes, there's only one now...I'm in a
    building process, and I need to get the first one to read first.).
    However, it's only going through 1 time and it should be 3 (there are
    three lines in the file with sp as the first field). What am I doing
    wrong?! Thanks.

    int code;
    code = readInFirstChars(fin);
    while (!fin.fail())
    {
    switch (code)
    {
    case 1:
    {
    readInASpinnerbait(fin, spinList);
    }
    default:
    {
    fin.ignore(80, '\n');
    }
    }
    code = readInFirstChars(fin);
    }

    int readInFirstChars(ifstream &fin)
    {
    char first;
    char second;
    int code = 999;
    char junk = '/';

    first = fin.get();
    if (first == 's')
    {
    second = fin.get();
    }
    if (second == 'p')
    {
    code = 1; // 1 is for Spinnerbait
    return code;
    }
    else if (second == junk)
    {
    fin.ignore(80, '\n');
    return code;
    }
    return code;
    }
     
    Francis Bell, May 25, 2004
    #1
    1. Advertising

  2. "Francis Bell" <> wrote...
    > Hello, I've got a 25 line file with lines of data like this:
    > sp/spinnerbait/AAA Lures/Mad Phil/silver/bass/1/1
    > The first field is the code that determines what to do. I need to loop
    > through this data file and, based on that first field, execute different
    > cases in a switch statement (yes, there's only one now...I'm in a
    > building process, and I need to get the first one to read first.).
    > However, it's only going through 1 time and it should be 3 (there are
    > three lines in the file with sp as the first field). What am I doing
    > wrong?! Thanks.
    >
    > int code;
    > code = readInFirstChars(fin);
    > while (!fin.fail())
    > {
    > switch (code)
    > {
    > case 1:
    > {
    > readInASpinnerbait(fin, spinList);
    > }
    > default:
    > {
    > fin.ignore(80, '\n');
    > }
    > }
    > code = readInFirstChars(fin);
    > }
    >
    > int readInFirstChars(ifstream &fin)
    > {
    > char first;
    > char second;
    > int code = 999;
    > char junk = '/';
    >
    > first = fin.get();
    > if (first == 's')
    > {
    > second = fin.get();
    > }


    So, if the first isn't an 's', it will never attempt to get the 'second'
    from the file, and so the following comparison will attempt to compare
    the contents of the _uninitialised_ 'second' variable to 'p'. How do
    you suppose that's going to work?

    > if (second == 'p')
    > {
    > code = 1; // 1 is for Spinnerbait
    > return code;
    > }
    > else if (second == junk)
    > {
    > fin.ignore(80, '\n');
    > return code;
    > }
    > return code;
    > }
    >


    Take a proper line and walk through your code doing what your program
    would do, and then proceed with the second line (as you should), and
    see what it's going to do. Correct the behaviour as you see fit.

    Victor
     
    Victor Bazarov, May 25, 2004
    #2
    1. Advertising

  3. Francis Bell

    Francis Bell Guest

    Victor Bazarov wrote:
    > "Francis Bell" <> wrote...
    >
    >>Hello, I've got a 25 line file with lines of data like this:
    >>sp/spinnerbait/AAA Lures/Mad Phil/silver/bass/1/1
    >>The first field is the code that determines what to do. I need to loop
    >>through this data file and, based on that first field, execute different
    >>cases in a switch statement (yes, there's only one now...I'm in a
    >>building process, and I need to get the first one to read first.).
    >>However, it's only going through 1 time and it should be 3 (there are
    >>three lines in the file with sp as the first field). What am I doing
    >>wrong?! Thanks.
    >>
    >>int code;
    >>code = readInFirstChars(fin);
    >>while (!fin.fail())
    >>{
    >> switch (code)
    >> {
    >> case 1:
    >> {
    >>readInASpinnerbait(fin, spinList);
    >> }
    >> default:
    >> {
    >>fin.ignore(80, '\n');
    >> }
    >> }
    >> code = readInFirstChars(fin);
    >>}
    >>
    >>int readInFirstChars(ifstream &fin)
    >>{
    >>char first;
    >>char second;
    >>int code = 999;
    >>char junk = '/';
    >>
    >>first = fin.get();
    >>if (first == 's')
    >> {
    >> second = fin.get();
    >> }

    >
    >
    > So, if the first isn't an 's', it will never attempt to get the 'second'
    > from the file, and so the following comparison will attempt to compare
    > the contents of the _uninitialised_ 'second' variable to 'p'. How do
    > you suppose that's going to work?
    >
    >
    >> if (second == 'p')
    >>{
    >> code = 1; // 1 is for Spinnerbait
    >> return code;
    >>}
    >> else if (second == junk)
    >> {
    >>fin.ignore(80, '\n');
    >>return code;
    >> }
    >>return code;
    >>}
    >>

    >
    >
    > Take a proper line and walk through your code doing what your program
    > would do, and then proceed with the second line (as you should), and
    > see what it's going to do. Correct the behaviour as you see fit.
    >
    > Victor
    >
    >

    Hello, and thanks. I wish I could say I understand what you are talking
    about, but I don't. I've been looking at this and walking through it
    for so long trying to figure out what needs to be done that I can't see
    straight. And I wish I knew what behavior to correct. But I don't.

    Frank
     
    Francis Bell, May 25, 2004
    #3
  4. * Francis Bell <> schriebt:
    > >
    > > Victor
    > >

    > Hello, and thanks. I wish I could say I understand what you are talking
    > about, but I don't. I've been looking at this and walking through it
    > for so long trying to figure out what needs to be done that I can't see
    > straight. And I wish I knew what behavior to correct. But I don't.


    Just do exactly as Victor says.

    Start at the first line in your program.

    Do what the computer will do with that (not what you wish it to do).

    Then the second line, perform it mechanically as the computer will.

    And so on.

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is top-posting such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, May 25, 2004
    #4
  5. > Hello, and thanks. I wish I could say I understand what you are talking
    > about, but I don't. I've been looking at this and walking through it
    > for so long trying to figure out what needs to be done that I can't see
    > straight. And I wish I knew what behavior to correct. But I don't.
    >


    It helps to indent your code properly to see this sort of problem, I've also
    added to comment to serve as labels

    int readInFirstChars(ifstream &fin)
    {
    char first;
    char second;
    int code = 999;
    char junk = '/';

    first = fin.get(); // A
    if (first == 's') // B
    {
    second = fin.get();
    }
    if (second == 'p') // C
    {
    code = 1; // 1 is for Spinnerbait
    return code;
    }
    else if (second == junk)
    {
    fin.ignore(80, '\n');
    return code;
    }
    return code;
    }

    Now you are fixated on the sp/ case, but imagine a line that doesn't start
    with s, say x instead.

    line A, reads 'x', assigns 'x' to first
    line B, first == 's' is not true, skip if statement, go to line C
    line C, what is the value of second? It has never been given a value, you
    have undefined behaviour here.

    You need a few more else's in your code to cover all the possibilities.

    if (first char is 's')
    {
    if (second char is 'p')
    {
    if (third char is '/')
    {
    }
    else
    {
    }
    }
    else
    {
    }
    }
    else
    {
    }

    But really this is madness, why did you stop using get(first, 4, '/')? get
    will work Seems like you are trying to code the behaviour of get by hand.
    Even better would be getline, but you were prejudiced against that for some
    reason.

    Do it like this, no messing

    string first;
    getline(fin, first, '/');
    if (first == "sp")
    {
    return 1;
    }
    else
    {
    fin.ignore(80, '\n');
    return 999;
    }

    Very simple, and you can easily add more cases as you develop your code.

    john
     
    John Harrison, May 25, 2004
    #5
  6. Francis Bell

    Dan Moos Guest

    The main point here is that you should do it on paper one line at a time.
    Assume nothing. Just take each line on its own merit, and do it on papre
    exactly. If you do this, the problem will jump out at you.

    you'd be amazed how often an elusive bug can be tracked down this way.
     
    Dan Moos, May 25, 2004
    #6
  7. Dan Moos wrote:
    > The main point here is that you should do it on paper one line at a time.
    > Assume nothing. Just take each line on its own merit, and do it on papre
    > exactly. If you do this, the problem will jump out at you.
    >
    > you'd be amazed how often an elusive bug can be tracked down this way.


    While it was my suggestion to step through the program one statement
    at a time, I can see where it can be difficult for somebody with limited
    knowledge of programming. No offense meant toward anyone, but some
    novice programmers cannot write code that works or cannot debug their
    code, often simply because they are not sure what the constructs they are
    to use actually do, or if they are sure (or think that they are sure),
    their understanding can easily be incorrect or incomplete.

    To the OP:

    In order to see what your program is in fact doing, you need to make
    sure you know what each statement does and what its side effects are.
    Even if you think you know what it does and what comes out of it, put
    that "knowledge" aside, take the book, and verify.

    The ability to "debug" your programs on a sheet of paper doesn't come
    natural to people. It's developed, just like any other skill. I wasted
    a lot of scrap paper in the beginning of my career (without symbolic or
    visual debuggers at my disposal at the time), but it did teach me to pay
    attention.

    Try to put aside your disgust with the non-working program, your
    frustration with your own inability to see the culprits (yet), and
    start anew. You could, of course, just write a new program altogether.
    It sometimes helped me, I remember. But it is rarely necessary. What
    you wrote is already probably close.

    Don't get annoyed because you have to look through it again and again,
    that's what programming is about (among other things), just like many
    other activities involving creative process and reaching a given goal.
    Don't get angry at the newsgroup when people in it do not just point
    out all your mistakes and tell you how to correct them. The whole idea
    of doing your own work is that in the future you don't have to ask for
    help as often because you learn. And believe me, learning by doing is
    much more effective than learning by listening.

    Good luck!

    Victor
     
    Victor Bazarov, May 25, 2004
    #7
  8. Francis Bell wrote:
    > Hello, I've got a 25 line file with lines of data like this:
    > sp/spinnerbait/AAA Lures/Mad Phil/silver/bass/1/1
    > The first field is the code that determines what to do. I need to loop
    > through this data file and, based on that first field, execute different
    > cases in a switch statement (yes, there's only one now...I'm in a
    > building process, and I need to get the first one to read first.).
    > However, it's only going through 1 time and it should be 3 (there are
    > three lines in the file with sp as the first field). What am I doing
    > wrong?! Thanks.
    >
    > int code;
    > code = readInFirstChars(fin);
    > while (!fin.fail())
    > {
    > switch (code)
    > {
    > case 1:
    > {
    > readInASpinnerbait(fin, spinList);
    > }
    > default:
    > {
    > fin.ignore(80, '\n');
    > }
    > }
    > code = readInFirstChars(fin);
    > }
    >
    > int readInFirstChars(ifstream &fin)
    > {
    > char first;
    > char second;
    > int code = 999;
    > char junk = '/';
    >
    > first = fin.get();
    > if (first == 's')
    > {
    > second = fin.get();
    > }
    > if (second == 'p')
    > {
    > code = 1; // 1 is for Spinnerbait
    > return code;
    > }
    > else if (second == junk)
    > {
    > fin.ignore(80, '\n');
    > return code;
    > }
    > return code;
    > }
    >


    I agree with John Harrison, in that you should
    be using a string and getline().

    However, strings cannot be used in a switch statement,
    and a ladder of if-elseif statements can get pretty
    ugly to read. A better result, IMHO, is to use
    a map of <string, function pointer>. You would use
    the string to find the appropriate function that
    would handle the rest of the input.

    Example:
    #include <istream>
    #include <string>
    #include <cstdlib>
    #include <map>
    using namespace std; // because I'm lazy.

    // Define a function pointer type.
    typedef void (*PTR_PROCESS_FUNC)(istream&);

    // Define a function for handling the "sp" case
    void Handle_Sp(istream& inp)
    {
    string text;
    getline(inp, text, '\n');
    cout << "Handling case for sp\n";
    cout << text;
    cout.flush();
    return;
    }

    // Define a map {associated array} type
    typedef map<string, PTR_PROCESS_FUNC> Process_Map;

    int main(void)
    {
    Process_Map func_map;

    // Initialize the map.
    // Associate the string "sp" with the Handle_Sp
    // function.
    func_map[string("sp")] = Handle_Sp;

    string text;
    istream inp("Your filename");
    while (getline(inp, text, '/'))
    {
    PTR_PROCESS_FUNC function_ptr;
    Process_Map::const_iterator iter;

    // Search for the function pointer associated
    // with the input string.
    iter = func_map.find(text);

    // Check if the text was found in the map.
    if (iter != func_map.end())
    {
    function_ptr = iter->second;
    function_ptr(inp); // Execute the function.
    }
    else
    {
    cerr << "No function associated with "
    << text
    << "\n";
    inp.ignore(1000, '\n'); // ignore the rest of the line.
    }
    } // End: while each line.

    return EXIT_SUCCESS;
    }

    --
    Thomas Matthews

    C++ newsgroup welcome message:
    http://www.slack.net/~shiva/welcome.txt
    C++ Faq: http://www.parashift.com/c -faq-lite
    C Faq: http://www.eskimo.com/~scs/c-faq/top.html
    alt.comp.lang.learn.c-c++ faq:
    http://www.raos.demon.uk/acllc-c /faq.html
    Other sites:
    http://www.josuttis.com -- C++ STL Library book
     
    Thomas Matthews, May 25, 2004
    #8
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Replies:
    4
    Views:
    565
    Chris Uppal
    May 5, 2005
  2. KK
    Replies:
    2
    Views:
    705
    Big Brian
    Oct 14, 2003
  3. MuZZy
    Replies:
    7
    Views:
    1,822
    Mike Hewson
    Jan 7, 2005
  4. addi
    Replies:
    0
    Views:
    292
  5. Isaac Won
    Replies:
    9
    Views:
    444
    Ulrich Eckhardt
    Mar 4, 2013
Loading...

Share This Page