Settings parser

[eXpl0it3r]

std::unordered_map?

It doesn't sort by keys, but it also doesn't retain an order. The order you'd get when iterating over a std::unordered_map depends (afaik) on the used hash function.

[Nexus]

That's why I will stick with the std::vector. I updated the documentation to clarify that keys have to be unique.

I would also check this precondition in the code with assertions.

assert(std::find(v.begin(), v.end(), value) == v.end());

When deciding about vector/map/unordered_map, choose the most simple or standard one to use (usually std::vector) unless you have specific reasons to deviate. Maps and even more hash maps can only play their advantages in the presence of many elements (100 are not many).

And please add a link to your initial post

[Foaly]

Alrighty I gave the class another overhaul. Now only the data nessessary is kept in memory. Keys are now unique and will be overwritten if used more than once. A std::map is used to ensure that and also because it is the easiest to find the keys. Writing modified data back to the file is still supported.
Also strings containing whitespace are now supported and there were quiet a few bug fixes
I created a git repo with the code here and I update the code on the wiki page.

edit: @Nexus: what kind of link do you want me to add in the first post? A link to the wiki page is already there.

[Nexus]

Oh sorry, my bad. I just scrolled up and looked at the first post, not realizing it wasn't the first page

Looking at your code, you seem to do a lot of manual work to extract strings, split and parse them. std::ifstream::operator>> is sometimes not flexible enough to do everything, but it could still simplify a lot if you use formatted I/O capabilities of the standard library.

I once wrote a simple parser for a text file where every line is either empty or contains a key-value pair according to the format key = value, with extremely few code:

std::ifstream file(filename);
std::map<std::string, double> map;

std::string key;
std::string assignment;
double value;
while (file >> key >> assignment >> value)
        map[key] = value;

It will be more complex to handle comments and special cases, but maybe this can still help as an inspiration.

[Foaly]

Wow that is indeed very few lines for a simple key value parser. Thanks for sharing the code.
I don't think it will work for my case though, because I want to support strings containing whitespace as a value type. And according to the doc the >> operator separates at whitespace, meaning I would only get the first word. I don't see a way to change that, so there seems to be no other way but std::getline().

[Nexus]

You can use the std::noskipws manipulator

[Foaly]

I investigated a little bit and unless I am missing something, I don't think this can help me. From what I understand std::noskipws only effects leading whitespace. But what I would need is to change the seperator for the >> operator. So I can parse something like that

Code: [Select]

key = some value containing spaceBut I don't think that's possible, is it?

[Laurent]

You already gave the solution: std::getline. What's wrong with it?

[Foaly]

There is nothing wrong with std::getline(), that's what I am using right now.
But Nexus suggested that he did something a little more elegant with >>. That's why I asked if it's possible to extract multiple words containing whitespace into one string.
But I guess that's not possible, so I'll stick with getline()

[Laurent]

Why would operator >> be more elegant than std::getline? The code logic can be the exact same, just with one function instead of the other.

[SeriousITGuy]

Because one would need less code to actually parse the line and store the key/value-pairs, like Nexus posted. But this comes with the limitation that you can actually not do any error or syntax checking in the text file, that's the reason why I use std::getline() and then a parse-function with 40 LOCs:

(click to show/hide)

void ConfigParser::parse( const std::string& line )
{
  // if line is empty or starts with the comment character return from parse
  if( line.size() == 0 || line[ 0 ] == '#' )
    return;
  // if line does not end with a ; report error and skip line
  if( line[ line.length() - 1 ] != ';' ) {
    std::cerr << "Malformed setting found in file " << _filename << " on line "
              << std::to_string( _lineIndex ) << ": " << line
              << "\nReason: No ';' found" << std::endl;
    return;
  }
  std::size_t equalSign = line.find( '=' );
  // when no '=' is found report error and skip line
  if( equalSign == std::string::npos ) {
    std::cerr << "Malformed setting found in file " << _filename << " on line "
              << std::to_string( _lineIndex ) << ": " << line
              << "\nReason: No '=' found" << std::endl;
    return;
  }

  // everything is ok, so parse the key-value pair and store it
  std::string key{}, value{};
  key = line.substr( 0, equalSign );
  trim( key );
  // make key always lower-case
  std::transform( key.begin(), key.end(), key.begin(), ::tolower );
  value = line.substr( equalSign + 1, line.length() - (equalSign + 1) - 1 );
  trim( value );

  // first look into data if 'key' is already present
  auto itr = _data.find( key );
  if( itr == _data.end() ) {
    // if not add key-value pair
    _data[ key ] = value;
  } else {
    // if yes report this error and skip the line
    std::cerr << "Duplicate key " << key << " found in file " << _filename << " at line "
              << _lineIndex << ". Skipping value" << std::endl;
    return;
  }  
}

This way it is still a simple parse-function (which could be expanded upon) but with a robust use.

@Foaly: You could make your code more readable and less typing-error prone if you would use auto, e.g.
auto it = m_data.find(key);
instead of
std::map<std::string, std::string>::const_iterator it = m_data.find(key);

These dumb iterator-types are actually one of the best reasons why auto even exists.

[Foaly]

Hi there!
The settings parser class has gone through some changes recently. I finally had some time to wrap it all up nicely and update the wiki page.
The class uses templates now and features things like reading/writing values from/to a vector. Parsing has been moved to a seperate function and is now more robust. Lots of small things (like using auto) have changed. For a full changelog just refer to the commit history of the repo.
All the things mentioned in this thread should be fixed. So check it out and let me know what you think!

[Laurent]

A few comments:

1. The string <--> T conversion functions should definitely not be part of the public header. They should be in their own private header, included in the .cpp only.

2. Why did you split the load and save functions in two parts (one tiny and one that does all the job)?

3. Is the locale member useful? Isn't the one argument version of std::isspace using the default locale anyway?

[wintertime]

You don't initialize all members in the constructor. That means the locale gets default-constructed from the currently active global locale that someone could have set to anything: http://en.cppreference.com/w/cpp/locale/locale/locale . Then the stream could use a different locale (internally copied from the global locale at time of its construction) than your later calls to isspace, if it got changed inbetween.
It would be best to not use any function that can depend on a locale (especially when parsing files that should never depend on one), but if you can't help it explicitely construct a "C" locale and use it for everything.

I also don't like how you read and parse the file again on a therefore mislabeled write method, possibly using a different locale and if the file got changed meanwhile you mix in some possibly old settings.
There I would just read the whole file in binary mode into memory with a single read call to the stream inside your read method, do in memory parsing and keep all data for writing it later without reading it again.
Then you could also have a save method taking a filename to make it also useful for the case of writing a copy of the settings somewhere else, to prevent partial overwriting on errors or have a backup or construct a new settings file without loading it first.

[underww]

You should handle an exception about a line with only white spaces in the parseLine function.