Attempting to modify this function to read from a (;) semi-colon-separated file

I have a function reads from a file like this

file

1
2
3
foo;bar
foo;bar
foo;bar


function

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
void EntryList::loadfile(const char filefoo[])
{
	ifstream		in;
	char			foo[MAX_CHAR];
	char			bar[MAX_CHAR];
	AddressEntry	anEntry;

	in.open (filefoo);
	if(!in)
	{
		in.clear();
		cerr << endl << "Fail to open " << filefoo << " for input!" << endl << endl;
		exit(1);
	}

	in.get(foo, MAX_CHAR, ';');
	while (!in.eof())
	{
		in.get();						//remove field seperator ';'			
		in.get(bar, MAX_CHAR, '\n');
		in.ignore(100, '\n');				//remove record seperator '\n'

		anEntry.setfoo(foo);
		anEntry.setbar(bar);

		addEntry(anEntry);

		in.get(foo, MAX_CHAR, ';');		        //start the next record
	}
	in.close();
}


and I would like to modify it
so that it reads from a file like this

new file

1
2
3
foo;bar;foobar
foo;bar;foobar
foo;bar;foobar


modified function

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
//--------------------------------------------
// Function: loadAddressBook(const char fileName[])
// Purpose: reads the entries from the file and adds them to the list
// Returns: N/A
//--------------------------------------------

void EntryList::loadfile(const char fileName[])
{
    ifstream        in;
    char            foo[MAX_CHAR];
    char            bar[MAX_CHAR];
    char            foobar[MAX_CHAR];
    TaskList        theEntry;

    in.open(fileName);
    if (!in) {
        in.clear();
        cerr << endl
        << "Failed to open "
        << fileName
        << " for input!" << endl << endl;
        exit(1);
    }

    in.get(foo, MAX_CHAR, ';');
    while (!in.eof())
    {
        in.get(); // rm ;
        in.get(bar, MAX_CHAR, ';');
        in.get(foobar, MAX_CHAR, '\n'); // rm '\n'
        in.ignore(100, '\n');

        theEntry.setfoo(foo);
        theEntry.setbar(bar);
        theEntry.setfoobar(foobar);

        addEntry(theEntry);

        in.get(foo, MAX_CHAR, ';');

    }

    in.close();

}


I am in the middle of rewriting this program for at least the 4 time.
and I have modified the file how I (humanly) think I should
to this. I have had issues in the past, doing it this way. (still working on the other parts of the program so I cannot be too specific right now, but I know my results were unexpected ) So my question is does the function that I modified look correct for what I am trying to do? Am I off by one? I guess I am struggling with understanding how the original function is working. (step by step systematically.) hence my confusion about my modified function.

I can also provide any other functions you would like to take look at, my setters and getters. Also if you have any questions or feedback I would appreciate it greatly.
Last edited on
I posted over on stack overflow, and I have received this response.
as a better way to read the file.
It makes more sense to me procedurally.


http://stackoverflow.com/questions/1...ect=1#19882576

Here is the solution I was provided with (still untested.)

"Both the orginal and the modified functions are wrong: you alway need to check for successful input after trying to read something (I guess, if I eventually pass away my headstone will have that engraved...). Using in.eof() to control an input loop in general does not work!

If the line ends in a string with less than MAX_CHAR characters, the next line get ignored: you need to check if the input ends in a newline character and, if not, ignore the remaining character. If the last input ends with a newline character, you don't want to ignore characters. Also, if the line happens to end in a string with more than 100 characters, it also doesn't work. The magical constant for std::istream::ignore() to ignore as many characters as necessary is, inconveniently, spelled std::numeric_limits<std::streamsize>::max() and is declared in the header <limits>.

Basically, your loop should start with"

1
2
3
4
5
6
7
8
9
10
11
while (in.getline(foo, MAX_CHAR, ';')
         .getline(bar, MAX_CHAR, ';')
         .get(foobar, MAX_CHAR, '\n')) {
    if (foobar[in.gcount() - 1] == '\n') {
        foobar[in.gcount() - 1] = '\0';
    }
    else {
        in.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    }
    // process the inputs
}


"The code uses std::istream::getline() for the first two components to avoid the separate to be stored: it is sufficient if the separator is extracted and the input is stopped. For the last component std::istream::get() is used because it is necessary to verify if the last character stored was a newline. The last character stored is access using std::istream::gcount() which contains the number of characters stored by the last unformatted input function. Since the input succeeded and it would stop either when storing a newline or after storing MAX_CHAR characters, in.gcount() - 1 is always a valid index. Note, however, that the code is not tested..."


I guess it is safe to mention that, the thing that is confusing the most is..
The first instance

 
 in.get(foo, MAX_CHAR, ';');


in the provided function make no sense to me, why it is outside the while loop. It seems it is operating on some side effect that I am unfamiliar with, and really leaves me at a loss when, I think about modifying the file to have a second ';' as a delimiter.

Still open for feedback and input.

:)
It is because you should not loop on EOF. That is a bad practice.

Consider why:

1
2
3
4
5
6
7
8
while (!f.eof())
  {
  f.getline( s, N ); // try to get input

  // (what if input failed?)

  do_something_with( s ); // wait... but what if input failed?
  }

It is better to try to get input, then test for failure in the loop.

1
2
3
4
5
6
7
8
while (true)
  {
  f.getline( s, N ); // try to get input

  if (f.fail()) break; // If EOF or some other error, we're done.

  do_something_with( s ); // fine, if we get this far, we know input succeeded
  }

C++ makes this kind of thing easy, since the stream input operators typically return the stream object itself, so you can put it all in the while condition:

1
2
3
4
while (f.getline( s, N ).fail()) // try to get input AND test for EOF or error
  {
  do_something_with( s ); // fine, if we get this far, we know input succeeded
  }

The standard stream objects automatically return good or bad -- you don't have to explicitly call fail():

1
2
3
4
while (f.getline( s, N ))
  {
  do_something_with( s );
  }

The code you were given is chaining calls to get input the same way.

Hope this helps.
Topic archived. No new replies allowed.