Parsing text file in C++

Question

I have a text file that contains name, age, salary, hoursWorked, randomText and are filled with different delimiters.

Text file:

susan:25-2600,28[asd]
mary:21-2200,38[asd]
john:23-3400,46[asd]

Instead of breaking them into individual strings using the code shown below:

string name,age,salary,hoursWorked,randomText;
ifstream readFile("textfile.txt");
while(getline(readFile,line)) {
 stringstream iss(line);
 getline(iss, name, ':');
 getline(iss, age, '-');
 getline(iss, salary, ',');
 getline(iss, hoursWorked, '[');
 getline(iss, randomText, ']');
}
readFile.close();

What are some better strategies other than coding it this way?

Side note

I declared all the variables to strings because of the getline() method.

Answer 1

I would create a class and define an input operator:

struct Person
{
 std::string name;
 std::string age;
 std::string salary;
 std::string hoursWorked;
 std::string randomText;
 friend std::istream& operator>>(std::istream& str, Person& data)
 {
 std::string line;
 Person tmp;
 if (std::getline(str,line))
 {
 std::stringstream iss(line);
 if ( std::getline(iss, tmp.name, ':') && 
 std::getline(iss, tmp.age, '-') &&
 std::getline(iss, tmp.salary, ',') &&
 std::getline(iss, tmp.hoursWorked, '[') &&
 std::getline(iss, tmp.randomText, ']'))
 {
 /* OK: All read operations worked */
 data.swap(tmp); // C++03 as this answer was written a long time ago.
 }
 else
 {
 // One operation failed.
 // So set the state on the main stream
 // to indicate failure.
 str.setstate(std::ios::failbit);
 }
 }
 return str;
 }
 void swap(Person& other) throws() // C++03 as this answer was written a long time ago.
 {
 swap(name, other.name);
 swap(age, other.age);
 swap(salary, other.salary);
 swap(hoursWorked, other.hoursWorked);
 swap(randomText, other.randomText)
 }
};

Now your code looks like this:

Person data;
while(readFile >> data)
{
 // Do Stuff
}

PS. I noticed you were using string and ifstream without the std::. This suggests you have using namespace std; in your code. Please don't do that. see Why is "using namespace std;" considered bad practice?

Don't explictly close() a file unless you are going to check that it worked (or are going the re-open). Prefer to let the destructor do the closing (that way it is exception safe). See: My C++ code involving an fstream failed review

• Praxeolitic: In the operator>> definition I needed to add the class before the member variables (e.g. data.name, data.age) in the getline calls for this to compile.
• Loki Astari: @user39469: Sorry. Fixed.
• utnapistim: Nice. The only thing I would change: I would use a local Person tmp; instance, and in the if code block (the "OK: All read operations worked" part) I would write data = std::move(tmp); (or std::swap(data, tmp);).. This way, you avoid the case when the stream doesn't actually contain a valid person (in that case, if one of the getline calls fails, data will not be partially filled with garbage).