Settings parser

[Nexus]

I agree... If you read the links I've posted, you'll see that both of your objections are addressed there

Declaring parameters const is meaningful for references and pointers, but on the top-level (like const sf::Vector param) it clutters the interface with irrelevant information.

One can follow your suggestions in the implementation (function definition), but never in the interface (declaration).

[select_this]

Mine wasn't an objection, mine was a 'let's clarify before someone reads this thread and doesn't bother to follow the links'

[Jesper Juhl]

No objections here either. Just wanted to point out some details 

[Foaly]

Aah damn! I knew something felt odd, while I was doing this... But it was early in the morning and I thought I simply forgot the const. Didn't think there for a minute
Thanks for pointing this out. And also thanks for the links! I didn't know you could make the parameters in the implementation const but not in the interface. Very interessting.
Anyway I removed the const from the interface.

[SeriousITGuy]

Just stumbled across the settings parser on the wiki, and I have a question regarding the data structure you save the data in. Why do you use a std:vector<std::pair<std::string, std::string>>  ?
With a vector of pairs you can enter duplicate settings, and your code then will always only select the value with the lower index-key in your vector. The second value with the same key (but higher index in your vector) will never get selected (and shouldn't be there in the first place in my opinion).

That's a little design problem and I think this should be addressed. If a std::map<std::string, std::string> is used instead of a vector, only one instance of a key can live inside the map, which gets overwritten if the same key is present in the text file (which could also be eliminated easily using a map).

Also maybe some checking if the given text file yields a valid format maybe benefical, if the user manually edits the text-file (which developers often do for unit testing). Some points from the top of my head:
- Left- and right-trimming the lines for whitespace before parsing them
- converting the key to be always lower (or upper) case, so that it's really unique (so case-variants of identical keys can also be detected)

Throwing an exception instead of returning false if the text file cannot be read may also be better, a bool-return can be ignored, a thrown exception not

Just some improvement thoughts to make the already good implementation more robust.
Cheers!


OT-Question:

I agree that it's useless as part of the interface, but it can sometimes be useful to do it in the implementation.
Like so:

void someFunc(int i);

void someFunc(const int i)
{
... do stuff involving 'i' ...
}

How do you do this? If I declare a function like you without the 'const' and then define it with 'const', I get a C2511: Overloaded member function not found in '...'
I see the benefit of it, but can't implement it. Confused here 

[Nexus]

How do you do this? If I declare a function like you without the 'const' and then define it with 'const', I get a C2511: Overloaded member function not found in '...'

What compiler? Do you have a minimal complete example?

According to the C++ standard, top-level CV qualifiers do not contribute to the parameter type. That is,

void function(int i);
void function(const int i);

are the same signatures. This, of course, does not apply to const-references, const-pointers or a memberfunction const at the end referring to this.

[SeriousITGuy]

This, of course, does not apply to const-references

Haha, that's it, was working with references and did not notice its pointed towards copy-values. Thanks again

[Foaly]

Thanks! Those are really good point. But since I am on vacation, I can't investigate your suggestions… I will look into it when I get back 

[SeriousITGuy]

I took the liberty of coding something myself with your code as a starting point, and worked in my thoughts.
Here is my version of a very simple config file parser https://github.com/SeriousITGuy/ConfigParser

[Jesper Juhl]

For simple config files I personally just use libconfig. It has a nice C++ API, does all I need (and more than I could be bothered to implement on my own), is free, had a permissive license and works very well.

[SeriousITGuy]

Yeah, I know libconfig, already worked with it, it is indeed very good and just does the job.
I wrote my own 'very simple' parser just for the sake of simplicity, and for use it in my future projects because only thing I need is a simple key=value parser. If one day I need something elaborate like the compound config settings libconfig is able handling of, I will choose libconfig for sure

Just wanted to share with the SFML-Comm because it shared so much with me. 

[Jesper Juhl]

Thank you for sharing 

[Foaly]

Hello everybody!
Like I said I was away for a bit, but I found some time to look at the code again.
I thought about the suggestion that Jesper Juhl made (using a std::map instead of a std::vector, to ensure unique keys), but since this class also supports changing the values and writing them back to the file, the order of insertion is important and needs to be preserved. That's why I will stick with the std::vector. I updated the documentation to clarify that keys have to be unique.
I also refactored the read() method to be more robust handling whitespace and I rearranged some includes.

[dabbertorres]

You could maybe use a map AND a vector. Have the map contain all the keys and values, and have the vector store all the keys, thereby keeping order. That way you get the benefits of both (order and unique keys), and I don't think it would make it that much more complex.

[p5ych0]

std::unordered_map?