Splitting and storing config file lines

Question

I wrote the following code to read a config file line by line and split each line into 2 parts and store them separately. The split is done on the basis of a delimiter =:

std::ifstream configfile(path_string.c_str());
std::string line;
std::vector<std::string> linesOfConfigFile;
while (std::getline(configfile, line))
 linesOfConfigFile.push_back(line);
configfile.close();
std::string currentItem;
char delimeter = '=';
std::vector<std::string> RHS;
std::vector<long long> LHS;
bool flag = false;
for (int i =0; i<linesOfConfigFile.size(); i++)
{
 std::stringstream singleLine(linesOfConfigFile[i]);
 while(std::getline(singleLine, currentItem, delimeter))
 {
 if(flag == true) 
 {
 RHS.push_back(currentItem);
 flag = false;
 }
 else if (flag == false)
 {
 long long orderbookId = 0;
 <some library class>::toNumeric(currentItem, orderbookId);
 LHS.push_back(orderbookId);
 flag = true;
 }
 }
}
for(int i=0; i<RHS.size(); i++)
 StorageMap.insert( std::pair<long long, std::string>( LHS[i], RHS[i]) );

The review comment I got was this:

The below code can be written in a better way, Need not use a Boolean flag to differentiate RHS and LHS. The outer for loops should be sufficient.

I cannot understand what difference he wishes to have. Can someone please help me?

• Gareth Rees: Have you tried asking him?
• amon: As an aside: when branching on booleans, you don't have to explicitly test whether they're true or false. Instead: if (flag) { ... } else { ... } flag = !flag. Note that flipping flag state is a common operation and can be done outside the conditional.
• Will Ness: what you did is an Antipattern: you perform a test with predictable outcome. You know that first comes LHS, and then comes RHS. Your code should reflect this explicitly, not pretend that LHSs and RHSs come in all kinds of different orders. They don't. The change here would be loop unrolling - performing two getlines on each step, first with a "=" delimiter, the second without it. Thus the code will explicitly and clearly reflect the nature of the situation; the knowledge won't be hidden in the obscure mechanics of getline stopping on end-of-line too, having failed 2 find the delim.
• Chani: @WillNess Thanks, why don't you add that as an answer.

Answer 1

I wrote the following code to read a config file line by line and split each line into 2 parts and store them separately.

So there are two parts on each line, the stuff before the =, which is stored as a string, and the stuff after the =, which shall be parsed as an integer and stored as a long long.

Thus you can handle each line on its own completely, without using a flag to determine at which part of the line you currently are:

for (int i =0; i<linesOfConfigFile.size(); i++)
{
 std::stringstream singleLine(linesOfConfigFile[i]);
 // get first part of the line
 std::getline(singleLine, currentItem, delimeter);
 // store it
 RHS.push_back(currentItem);
 // Now get the next part of the line
 std::getline(singleLine, currentItem, delimeter);
 // parse it
 long long orderbookId = 0;
 <some library class>::toNumeric(currentItem, orderbookId);
 // and store the result
 LHS.push_back(orderbookId);
 // done with this line :D
}

This supposes of course that the config file is well-formed, and the format requires that each name = value is on a separate line.

If the config file is malformed, or the file format is not as simple, this version may fail where yours wouldn't (for example

name =
value

would work with yours), but neither version does robust error handling, and both would fail on most malformed inputs. Config files can generally be assumed to follow the required format, so the absence of error handling probably isn't a serious problem.

• Chani: when I call std::getline(singleLine, currentItem, delimeter); for the second time; wont it again return me the same string it returned the first time ?
• Daniel Fischer: No, singleLine is a stringstream, it keeps trace of what it has already used, and delivers the next item each call (while there is one, of course).