Can anybody tell me what I did wrong (cpp) formatting with getline

Question

trying to format with c++ getline function. The output puts everything at the first record number forename instead of where it should go.

Code:

#include <fstream>
#include <string>
#include <iostream>
using namespace std;

int main()
{
    const int RANGE = 12;
    string tab[RANGE];
    int i = 0, j = 0;
    ifstream reader("records.txt");
    if (!reader)
    {
        cout << "Error opening input file" << endl;
        return -1;
    }
    while (!reader.eof())
    {
        if ( ( i + 1) % 4 == 0)
            getline( reader, tab[i++], '\n');
        else
            getline( reader, tab[i++], '\t');
    }
    reader.close();
    i = 0;
    while (i < RANGE)
    {
        cout << endl << "Record Number: " << ++j << endl;
        cout << "Forename: " << tab[i++] << endl;
        cout << "Surname: " << tab[i++] << endl;
        cout << "Department: " << tab[i++] << endl;
        cout << "Telephone: " << tab[i++] << endl;
    }
    return 0;
}

Contents of TXT file:

John    Smith    Sales    555-1234
Mary    Jones    Wages    555-9876
Paul    Harris   Accts    555-4321

Please run the code for yourself to understand what happens and put the txt file in the same folder as your code.

Hope someone can help me thanks.

It looks like you may want the extraction operator (`>>`) instead of `getline` here. ie: `reader >> tab[i++];` — scohe001, Jan 30 '20 at 21:29
please include the output in the question. What I got seems to be different from how others interpret the question — 463035818_is_not_a_number, Jan 30 '20 at 22:09

score 1 · Answer 1 · answered Jan 30 '20 at 21:45

See Why is iostream::eof inside a loop condition (i.e. while (!stream.eof())) considered wrong?.

Also, your final while loop should only output the strings that were actually read into the array, not the full array, if the file has less than 12 strings. But unless you can guarantee that your file never exceeds 12 strings, you should use std::vector instead of a fixed array.

Also, instead of alternating the getline() delimiter in a single loop, I would just use an outer loop to read whole lines only, and then separately read tab-delimited values from each line. And then store the values in an array/vector of struct instead of individually.

Try something more like this:

#include <fstream>
#include <sstream>
#include <string>
#include <iostream>
#include <vector>
using namespace std;

struct Person
{
    string foreName;
    string surName;
    string department;
    string phoneNumber;
};

int main()
{
    ifstream reader("records.txt");
    if (!reader)
    {
        cout << "Error opening input file" << endl;
        return -1;
    }

    vector<Person> people;
    string line;

    while (getline(reader, line))
    {
        istringstream iss(line);
        Person p;
        getline(iss, p.foreName, '\t');
        getline(iss, p.surName, '\t');
        getline(iss, p.department, '\t');
        getline(iss, p.phoneNumber, '\t');
        people.push_back(p);
    } 

    reader.close();

    int j = 0;
    for (Person &p : people)
    {
        cout << endl << "Record Number: " << ++j << endl;
        cout << "Forename: " << p.foreName << endl;
        cout << "Surname: " << p.surName << endl;
        cout << "Department: " << p.department << endl;
        cout << "Telephone: " << p.phoneNumber << endl;
    }

    return 0;
}

anastaciu · Accepted Answer · 2020-01-30T22:10:22.623

There are easier ways to separate words in an istream, namely C++ sring stream tools:

#include <fstream>
#include <iostream>
#include <sstream>  //<-- string stream library

using namespace std; //<-- should not be used, use scope std::

int main() {

    const int RANGE = 12;
    string tab[RANGE];
    string temp;       //<--to store each field temporarily
    int i = 0, j = 0;
    ifstream reader("records.txt");
    if (!reader) {
        cout << "Error opening input file" << endl;
        return -1;
    }
    while (getline(reader, temp)) { //<-- read one full line
        stringstream ss(temp); // <-- input to a string stream
        while(ss >> tab[i]){   // <-- passing strings to the string array one by one
            i++;
        }
    }
    reader.close();
    i = 0;
    while (i < RANGE) {
        cout << endl << "Record Number: " << ++j << endl;
        cout << "Forename: " << tab[i++] << endl;
        cout << "Surname: " << tab[i++] << endl;
        cout << "Department: " << tab[i++] << endl;
        cout << "Telephone: " << tab[i++] << endl;
    }
    return 0;
}

The idea here was to mess as little as possible with your code, one thing I would advise is to use std::vector instead of normal fixed size arrays. Also, as it was said and linked, eof is very unreliable.

score 0 · Answer 3 · answered Jan 30 '20 at 21:45

The source of your problem, I think, is explained in Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?.

You are reading into tab[12], tab[13], tab[13], and tab[14] due to that error. Of course, that leads to undefined behavior.

Change the loop to:

// Read the contents of the file line by line
std::string line;
while (getline( reader, line))
{
   // Process each line's contents.
   std::istringstream str(line);
   getline(str, tab[i++], '\t');
   getline(str, tab[i++], '\t');
   getline(str, tab[i++], '\t');
   getline(str, tab[i++], '\n');
}

Make sure to add

#include <sstream>

To be doubly sure that you are not using the array using out of bounds indices, add a check.

while ( i+4 < RANGE && getline( reader, line))
{
   ...
}

score 0 · Answer 4 · answered Jan 30 '20 at 22:04

First, while (!reader.eof()) is not doing the right thing.

The immediate problem you see is caused by the fact that your file does not contain '\t', hence already the very first getline reads all the contents of the file into tab[0]. (At least thats what I got after 1-to-1 copying your file contents)

Your code is rather difficult, because you declare variables long before you use them and later reuse them. You have a fixed size array, but when there are more lines in the file your code will just crash. Also reading everything into a plain array of strings is making things complicated. Accessing forename or other fields requires you to compute the offset into the array. Better use a data structure:

struct file_entry {
    std::string first_name;
    std::string last_name;
    std::string departure;
    std::string phone;
};

Then you can define an input operator:

std::istream& operator>>(std::istream& in,file_entry& fe) {
    return in >> fe.first_name >> fe.last_name >> fe.departure >> fe.phone;
};

And use a std::vector to store as many entries as there are in the file:

int main() {
    std::string contents{"John    Smith    Sales    555-1234\n"
                         "Mary    Jones    Wages    555-9876\n"
                         "Paul    Harris   Accts    555-4321\n"};

    std::stringstream reader{contents};
    std::vector<file_entry> data;    
    std::string line;
    while (std::getline(reader,line)) {
        file_entry fe;
        std::stringstream{line} >> fe;
        data.push_back(fe);
    }

    for (const auto& fe : data) {
        std::cout << "Forename: " << fe.first_name << '\n';
        std::cout << "Surname: " << fe.last_name << '\n';
        std::cout << "Department: " << fe.departure << '\n';
        std::cout << "Telephone: " << fe.phone << '\n';
    }
}

live example

PS you do not need to call close on the file, this is already done in its destructor. Not calling it explicitly has the benefit that the same code that works for a file stream also works for a stringstream.

Can anybody tell me what I did wrong (cpp) formatting with getline

4 Answers4