user2655324
user2655324

Reputation:

Entering in an infinite loop while reading a file

This code accepts student name, father's name, roll no and age from input file and put it in a presentable manner in output file.

In this code, when contents of input file are:

Vicky
Mohan
20094567
22   Ricky
Rahul
20091234
21

It works fine.

But if they are:

Vicky
Mohan
20094567
22
Ricky
Rahul
20091234
21

It enters into an infinite loop. Any suggestions??

ifstream inps("input", ios::in);
outs.open("output",ios::app);

string line;
int data,count=1;

for(getline(inps,line);line!="";getline(inps,line))
{
    count++;

    s1.setName(line);
    getline(inps,line);
    s1.setFatherName(line);
    inps >> data;
    s1.setRollNo(data);
    inps >> data;
    s1.setAge(data);

    outs.open("output",ios::app);
    outs << "Student name: " << s1.getName() << endl;
    outs << "Father’s name: " << s1.getFatherName() << endl;

    outs << "Roll number: " << s1.getRollNo() << endl;
    outs << "Age: " << s1.getAge() << endl << endl;
}

inps.close();
outs.close();

Upvotes: 1

Views: 3468

Answers (3)

James Kanze
James Kanze

Reputation: 153909

The reason for the symptoms you describe is that you're mixing formatted input with getline. There's also a fundamental problem that you never check whether any of the input succeeds.

The real problem manifests itself after the inps >> data lines: these lines skip whitespace and read an int, and no more. In particular, they leave any trailing whitespace, including the '\n' character, in the stream. So in your second case of input, after reading 22, there is still a '\n' in the stream, which will terminate the next call to getline (which instead of reading " Ricky", will read ""). This causes the input to become unsynchronized, which shortly results in your doing inps >> data when the stream is positionned at "Rahul". Trying to read an int when the input is "Rahul" fails, and failure is sticky; it will remain until you reset it, and all further attempts are no-ops. Since you've already read something into line once, it won't ever become empty, and you loop forever, doing nothing.

The first, and most important change is to check after every input that the input succeeded, and not try to read further if it hasn't. (The structure of your file is such that you probably can't reliably resynchronize if there is an error. Otherwise, it is a good policy to try and resynchronized, and continue, so that you can catch multiple errors in the input.)

The second thing you need to do is ensure that you read a complete line (including the '\n') when inputting integers. There are two ways of doing this: the classic way is to use getline, then initialize an std::istringstream with the line, and input the int using this. (This allows additional error checking, e.g. that there is no additional garbage in the line.) Alternatively, you can call inps.ignore( std::numeric_limits<std::streamsize>::max(), '\n' );, which will extract and ignore characters until '\n' (which is also extracted).

EDIT:

On rereading, it occurs to me that my text description isn't all that clear, so here's what happens in a step-wise explination:

  • The first time through the loop, everything works as expected, but the input position is immediately behind the "22" (which was the last input).

  • The getline at the top of the loop is called. It will return all of the characters between the "22" and the end of that line. If the "22" is immediately followed by a new line, this should result in an empty line, terminating the loop (although there is still more data to be read). If there are extra characters after the "22" (say a blank or so), then these will be read as the line.

  • Assuming there were extra characters, you then read " Ricky" as the father's name, and do inps >> data for the roll number on the string "Rahul". This fails, and sets the stream in an error condition, which causes all further operations to be no-ops.

  • So when you next reach the top of the loop, the getline is a no-op, the previous contents of line are unchanged, and you enter the loop again. And again, and again, because until you clear the error, all operations will be no-ops. All of the variables hold their old values.

The simplest solution is probably that suggested by Neil Kirk in a comment: read the entire file into an std::vector of lines, and parse those:

class Line
{
    std::string myContents;
public
    friend std::istream& operator>>( std::istream& source, Line& obj )
    {
        std::getline( source, obj.myContents );
        return source;
    }
    operator std::string() const { return myContents; }
};

// ...
std::vector<Line> lines( (std::istream_iterator<Line>( inps )),
                         (std::istream_iterator<Line>()) );

If you want to read the file on the fly, however (say because it might be too big to fit into memory, or simply because it is a good learning exercise):

while ( std::getline( inps, line ) && !line.empty() ) {
            //  but do you really what the second condition.
            //  if so, you should probably provide
            //  a function which will ignore whitespace.
    s1.setName( line );
    if ( std::getline( inps, line ) ) {
        s1.setFatherName( line );
    }
    if ( std::getline( inps, line ) ) {
        std::istringstream s( line );
        int data;
        if ( s >> data ) {
            s1.setRollNo( data );
        }
    }
    if ( std::getline( inps, line ) ) {
        std::istringstream s( line );
        int data;
        if ( s >> data ) {
            s1.setAge( data );
        }
    }
}

This is very succinct. It still needs additional error checking, and you probably want to keep track of the line number so that you can output it with any error message. But it shoul point you in the right direction.

EDIT2:

Also, you don't want to open the output file each time through the loop. Attempting to open an already open std::ofstream will fail, an as above, once the stream has failed, all further attempts to use it are no-ops.

Upvotes: 5

Some programmer dude
Some programmer dude

Reputation: 409176

It's because of how you read the input. You never actually check if it succeeds or not.

You need to do e.g.

while (std::getline(...))
{
    ...
}

Upvotes: 6

cpp
cpp

Reputation: 3801

Replace

for(getline(inps,line);line!="";getline(inps,line))

with

while (getline(inps, line))

Upvotes: 2

Related Questions