Loop problem, HELP please!

F

Francis Bell

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;
}
 
V

Victor Bazarov

Francis Bell said:
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
 
F

Francis Bell

Victor said:
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?




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
 
A

Alf P. Steinbach

* Francis Bell said:
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.
 
J

John Harrison

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
 
D

Dan Moos

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.
 
V

Victor Bazarov

Dan said:
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
 
T

Thomas Matthews

Francis said:
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
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,480
Members
44,900
Latest member
Nell636132

Latest Threads

Top