Skip to main content
Code Review

Return to Revisions

3 of 3
replaced http://stackoverflow.com/ with https://stackoverflow.com/

A couple of issues:

while (ifs.good())

Don't loop on good() (or eof()). This is one of the more annoying parts of the C++ iostream library. This should be:

while(std::getline(ifs, s, '0円'))

std::getline returns a reference to its input stream (std::basic_istream&). How this actually checks the stream state and continues looping is pretty complicated* - suffice to say this is the canonical way of doing it. If you're really being good about your error checking, you'd then call ifs.readstate() after your while loop and check the fail and error flags aren't set.

Secondly, this creates a new string every time around the while loop. Instead, you probably want:

//Note you probably shouldn't call it list, because of the std::list<T> type
vector<string> v; 
string s;
while(std::getline(ifs, s, '0円')) {
 v.push_back(s);
}

There are a few ways of signalling that an error has occurred - throwing an exception is one possibility. Returning a pair<vector<string>, bool> or pair<vector<string>, int> indicating either success (for bool) or number of characters read (for int) is another possibility - this is basically an "error code" way of dealing with the problem. Error handling is always tricky, unfortunately.

Either way, currently you return a vector whatever happens, so you might as well construct it outside the if test:

ifstream ifs(filename, ifstream::in);
vector<string> v;
if(ifs) { ... }
return v;
  • What happens that it calls the ifstream's operator void*() which tests all error flags and eof, returning a nullptr if any of them are set, otherwise a non-zero pointer. See also std::getline returns.
Yuushi
  • 11.1k
  • 2
  • 31
  • 66
default

AltStyle によって変換されたページ (->オリジナル) /