Simple C++ File I/O issue

Question

It's been a while since I've worked with File I/O in C++ (and just C++ in general) but I recently decided to use it to make a small console project for a friend. My issue is that I'm having some issues with a string array and File I/O (I'm not sure which is causing the problem). My code is as follows (ReadPSWDS is an ifstream):

                int i = 0;
            string str[200];

            ReadPSWDS.clear();
            ReadPSWDS.open("myPasswords.DoNotOpen");

            if(ReadPSWDS.is_open())
            {
                while(!ReadPSWDS.eof())
                {
                    getline(ReadPSWDS, str[i]); //Store the line
                    if(str[i].length()<1 || str[i] == "")
                    {
                        //Ignore the line if it's nothing
                    }
                    else
                    {
                        i++; //Move onto the next 'cell' in the array
                    }
                }
            }

            ReadPSWDS.close();

My issue is that on testing this out, the string array would appear to be empty (and on writing all those lines to a file, the file is empty as expected). Why is the string array empty and not filled with the appropriate lines of the text file?

Regards,

Joe

i'll advice not to direct coding of your friend's homework/assignment/project. you're not helping, you're killing him/her. — Donotalo, Jun 22 '11 at 06:46
Did you check that `ReadPSWDS.is_open()` is true? Always use printing statements (cout, or logging) to quickly verify that the machine reflects your intuitions. — amit kumar, Jun 22 '11 at 06:49
@Donotalo Heh. This is only a quick prototype that I've whipped up in a few minutes, I just can't understand why it isn't working. @phaedrus Yes, hence: if(ReadPSWDS.is_open()) — , Jun 22 '11 at 06:56
What happens in you print out `str[i]` right after the `getline`? — jonsca, Jun 22 '11 at 07:08
@Joe: still phaedrus' comment is valid: after getline, print out what you have read from the file. Also: prefer "while(std::getline(...))". It returns the stream that converts to false at eof or error (see http://stackoverflow.com/questions/2251433/checking-for-eof-in-stringgetline/2251612#2251612) — stefaanv, Jun 22 '11 at 07:09
The code you posted works, so the error must lie somewhere else. Either, as phaedrus suggested, the file isn't opened; or you do something else with the array which you did not post. What is the value of `i` after the loop? — carlpett, Jun 22 '11 at 07:49
Also, I found the line `ReadPSWDS.open("myPasswords.DoNotOpen")` slightly humorus :) — carlpett, Jun 22 '11 at 07:49
@carlpett I'm not doing anything else apart from what you see in the code I posted. So are you saying that it works absolutely fine for you? — , Jun 22 '11 at 14:42
@joe Yep. The only thing I had to add was declaration of ReadPSWDS, which I merged with the file opening; `ifstream ReadPSWDS("myPasswords.DoNotOpen");`. — carlpett, Jun 23 '11 at 08:20

score 6 · Accepted Answer · answered Jun 22 '11 at 08:02

6

The loop you've written is clearly wrong: you're testing eof() before failure, and you're not testing for failure after the getline. C++ I/O isn't predictive. (I can't be, since whether you're at eof() will depend on what you try to read.) The correct pattern would be:

while ( i < size(str) && getline( readSWDS, str[i] ) ) {
    if ( !str[i].empty() ) {
        ++ i;
    }

Note that I've added a test for i. As written, if your file contains more than 200 lines, you're in deep trouble.

I'm not sure that this is your problem, however; the loop as you've written it will normally only cause problems on the last line. (Typically, if the last line ends with a '\n', and is not empty, it will appear twice in your array.) Unless, of course, your file does contain more than 200 lines.

I might add that an even more typical idiom would be to make str an std::vector<std::string>, and write the loop:

std::string line;
while ( std::getline( readSWDS, line ) ) {
    if ( !line.empty() ) {
        str.push_back(line);
    }
}

This avoids having to define a fixed maximum number of lines.

answered Jun 22 '11 at 08:02

James Kanze

150,581
18
184
329

1

Correct. eof() is there to qualify the I/O error that happened, not to detect it. getline() return value has to be checked. – Benoît Jun 22 '11 at 08:05
1

@Benoît It's more complex than that. `eof()` (or rather the `eofbit`) is a bit ambiguous; on one hand, it is purely internal: it memorizes the fact that the streambuf has returned an EOF, so that the stream doesn't try to obtain additional characters once EOF has been seen. On the other, it's often used after failure to determine whether EOF or something else was the cause of failure, along the lines `if (src.bad()) /* hardware problem */ else if (!src.eof()) /* format error in text read */ else /* end of file */`. It's not 100% reliable for this, but it's the best we have. – James Kanze Jun 22 '11 at 08:19
Of course, vectors! I knew there was a better way but coded this late last night. As for eof(), I'm sorry for my incorrect usage - I don't work with files often and pulled the code from somewhere. I'll try your code, although I so no reason why my terrible original code didn't work. – Jun 22 '11 at 14:43
@Joesavage1 Except for the possibility that the input had more than 200 lines, I don't see anything off hand. – James Kanze Jun 22 '11 at 18:43

Simple C++ File I/O issue

1 Answers1