reading until the end of file in C++

Question

I'm trying to read till the end of a file for a phonebook app that im converting from C to C++. When I print the the results from the file i get this:

johnny smith
(Home)3
(Cell)4
x☺> x☺>
(Home)4
(Cell)4

it should print:

johnny smith
(Home)3
(Cell)4

Right now I'm using while(!infile.eof()) which i've read is a poor practice, but when I use infile.getline() I get a repeat of the first and last name, and the format is all jacked up. Is there anyway(or another way) to get rid of the junk at the end of the input or another way to read till the end of file in C++ that fixes this. I've been reading about different solutions, but the one a lot of sites seem to agree on is fgets, which is what I had with the original C version, but obviously fgets doesn't work with ifstream which is what I'm using. here is the code:

void contacts:: readfile(contacts*friends ,int* counter, int i,char buffer[],char    user_entry3[])
{
   ifstream read;
   read.open(user_entry3,ios::in);
   int len;
   contacts temp;
   *counter=0;
   i=0; 

     while (!read.eof()) { 
       temp.First_Name=(char*)malloc(36); 
       temp.Last_Name=(char*)malloc(36); 

       read>>temp.First_Name>>temp.Last_Name;

       read>>buffer;
       len=strlen(buffer);
       if(buffer[len-1]=='\n')
          buffer[len-1]='\0';

       temp.home=(char*)malloc(20); 
       strcpy(temp.home, buffer);

       read>>buffer;
       len=strlen(buffer);
       if(buffer[len-1]=='\n')
       buffer[len-1]='\0';


       temp.cell=(char*)malloc(20); 
       strcpy(temp.cell, buffer); 

      friends[i].First_Name=(char*)malloc(MAXNAME);
      friends[i].Last_Name=(char*)malloc(MAXNAME);
      friends[i].home=(char*)malloc(MAXPHONE);
      friends[i].cell=(char*)malloc(MAXPHONE);


  //adds file content to the structure
      strcpy(friends[*counter].First_Name,temp.First_Name);
      strcpy(friends[*counter].Last_Name,temp.Last_Name);
      strcpy(friends[*counter].home,temp.home);
      strcpy(friends[*counter].cell,temp.cell);


     (*counter)++;
     i++; 

   }
   //closes file and frees memory
    read.close();
    free(temp.Last_Name);
    free(temp.First_Name);
    free(temp.home);
    free(temp.cell);
}

The file is is simply `johnny smith` next line `3` next line `4`. The `` and `` is printed after the file gets read into the class and is simply the way I have it formatted. — , Nov 24 '12 at 18:40
This: `while (!read.eof())` is always wrong. Always. What if an error occurs? How do you catch it and stop reading? — Ed S., Nov 24 '12 at 18:43

score 8 · Answer 1 · edited Apr 10 '20 at 07:11

Don't use !eof(). It checks whether the last read failure was due to reaching the end of the file. It does not predict the future.
Don't use malloc in C++. If you do, check the return value for errors!
Don't use operator>> for char *. There's no size check so that's just asking for buffer overflows.
The '\n' check on buffer is useless. operator>> for strings stops at whitespace.
You're blindly strcpying a string of unknown length into temp.home of size 20. That's another buffer overflow.
... I kind of stopped reading there. If you want to read stuff from a file but stop on eof/error, you can do something like this:

.

string a, b, c;
while (true) {
    if (!(in >> a)) break;
    if (!(in >> b)) break;
    if (!(in >> c)) break;
    do_stuff_with(a, b, c);
}

score 7 · Answer 2 · answered Nov 24 '12 at 18:36

7

Do not use eof() to determine if you reached end of file. Instead, read what you want to read and then check if you successfully read the data. Obce reading failed you may use eof() to determine if the error is down to having reached the end of the file before producing an error report about a format error.

Since you mentioned that you read that using !infile.eof() is good practice: Can you point us at the source of this wrong information? This information need correction.

answered Nov 24 '12 at 18:36

Dietmar Kühl

150,225
13
225
380

1

My post says that I read it was POOR practice, not that it was good practice. – Nov 24 '12 at 18:46
Sorry, poor reading from my end. However, `if (infile.getline(buffer, n)) { ... }` should work just fine (although I'd recommend using `std::getline(in, str)` with a `std::string`. – Dietmar Kühl Nov 24 '12 at 19:09
what doest the `n` stand for in that `infile.getline`statement. forgive me for being a rookie lol. – Nov 24 '12 at 19:16
The `n` is the maximum number of `char`s you can store in `buffer`. Assuming `buffer` is declared as `char buffer[17];` it is `17` or `sizeof(buffer`)`. – Dietmar Kühl Nov 24 '12 at 19:19
ok, thats what I thought, and thats what I have been trying when I used the getline. My issue is that when I use getline my output looks like thus `3 4` next line` johnny smith` next line `johnny smith`. not only does it print out the name twice, but it obviously jacks up the formatting. – Nov 24 '12 at 19:23
Well, `std::istream::getline()` certainly does work and doesn't "jacks up" anything when used correctly. Since you don't show the code producing this problem I can't tell where things go wrong. Judging from the code you wrote, I'd recommend *strongly* that you throw out all uses of `malloc()`, `strcpy(), and `free()` and rather using the class `std::string`. If this is a homework assignment and you are not supposed to use this class, create your own simple string class instead (using `new char[n]`, `delete[] p`, etc.). – Dietmar Kühl Nov 24 '12 at 19:33

reading until the end of file in C++

2 Answers2

Linked