Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: Settings parser  (Read 31790 times)

0 Members and 1 Guest are viewing this topic.

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6286
  • Thor Developer
    • View Profile
    • Bromeon
Re: Settings parser
« Reply #15 on: July 29, 2014, 06:24:48 pm »
I agree... If you read the links I've posted, you'll see that both of your objections are addressed there ;)

Quote from: Nexus
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.
Quote from: Nexus
One can follow your suggestions in the implementation (function definition), but never in the interface (declaration).
« Last Edit: July 29, 2014, 06:28:27 pm by Nexus »
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

select_this

  • Full Member
  • ***
  • Posts: 130
  • Current mood: just ate a pinecone
    • View Profile
    • darrenferrie.com
Re: Settings parser
« Reply #16 on: July 29, 2014, 06:54:33 pm »
Mine wasn't an objection, mine was a 'let's clarify before someone reads this thread and doesn't bother to follow the links' ;)
Follow me on Twitter, why don'tcha? @select_this

Jesper Juhl

  • Hero Member
  • *****
  • Posts: 1405
    • View Profile
    • Email
Re: Settings parser
« Reply #17 on: July 29, 2014, 07:02:59 pm »
No objections here either. Just wanted to point out some details  :D

Foaly

  • Sr. Member
  • ****
  • Posts: 453
    • View Profile
Re: Settings parser
« Reply #18 on: July 30, 2014, 11:59:07 am »
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 :D
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

  • Full Member
  • ***
  • Posts: 123
  • Still learning...
    • View Profile
Re: Settings parser
« Reply #19 on: August 04, 2014, 09:53:25 am »
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  :o

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6286
  • Thor Developer
    • View Profile
    • Bromeon
Re: Settings parser
« Reply #20 on: August 04, 2014, 10:35:01 am »
Quote
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.
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

SeriousITGuy

  • Full Member
  • ***
  • Posts: 123
  • Still learning...
    • View Profile
Re: Settings parser
« Reply #21 on: August 04, 2014, 10:48:06 am »
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

  • Sr. Member
  • ****
  • Posts: 453
    • View Profile
Re: Settings parser
« Reply #22 on: August 05, 2014, 11:16:40 am »
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

  • Full Member
  • ***
  • Posts: 123
  • Still learning...
    • View Profile
Re: Settings parser
« Reply #23 on: August 27, 2014, 08:13:12 pm »
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

  • Hero Member
  • *****
  • Posts: 1405
    • View Profile
    • Email
Re: Settings parser
« Reply #24 on: August 27, 2014, 08:36:07 pm »
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

  • Full Member
  • ***
  • Posts: 123
  • Still learning...
    • View Profile
Re: Settings parser
« Reply #25 on: August 28, 2014, 09:45:39 am »
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

  • Hero Member
  • *****
  • Posts: 1405
    • View Profile
    • Email
Re: Settings parser
« Reply #26 on: August 28, 2014, 05:04:34 pm »
Thank you for sharing  :)

Foaly

  • Sr. Member
  • ****
  • Posts: 453
    • View Profile
Re: Settings parser
« Reply #27 on: September 18, 2014, 11:39:21 am »
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

  • Hero Member
  • *****
  • Posts: 506
    • View Profile
    • website/blog
Re: Settings parser
« Reply #28 on: September 18, 2014, 08:00:14 pm »
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

  • Guest
Settings parser
« Reply #29 on: September 18, 2014, 09:42:26 pm »
std::unordered_map?

 

anything