C++ questions

[lucasart]

Your code works, but I don't understand how!
Would you care to explain this C++ magic ?

Code: Select all

std::string(" ", !name.empty()) 

Calls std::string following constructor

string ( const char * s, size_t n );
    Content is initialized to a copy of the string formed by the first n characters in the array of characters pointed by s.

So if name is empty (when first part of the name is parsed) it is called std::string(" ", 0) that simply returns an empty string, otherwise it is called std::string(" ", 1) that returns a blank.

Very elegant!
I didn't know std::string had such a constructor. Now I can see why

[lucasart]

How do I find an element in a vector, with a given predicate ? I have a vector of options:

Code: Select all

struct Option
{
	enum Type { Boolean, Integer };
	Type type;
	std::string name;
	int value, min, max;
};

std::vector<Option> options;

Basically, I want to write this code in a more C++ idiomatic style, using std::find_if (or something similar)

Code: Select all

for (auto i = options.begin(); i < options.end(); i++) {
	if (i->name = "Hash" && i->type == Option::Boolean)
		break;
}

if (i < options.end()) {
	// do something with *i
}

[lucasart]

OK here's my attempt to solve the problem. In non technical terms, I want to:
* maintain a sorted list of options, which must always remain unique (according to a certain predicate)
* insert, and find options in this sorted list

From what I understood googling around, it seems that std::set is more appropriate than std::vector for that. Here is my attempt:

Code: Select all

#include <iostream>
#include <string>
#include <set>

struct Option {
	std::string name;
	enum Type {Boolean, Integer};
	Type type;
	int value, min, max;
	
	bool operator < (const Option& o) const
	{ return type < o.type && name < o.name; }
};

std::set<Option> options;

int main()
{
	// define a few options
	Option o1 = {"Hash", Option::Boolean, true, false, true};
	Option o2 = {"Hash", Option::Integer, 32, 1, 8192};
	Option o3 = {"Verbose", Option::Boolean, true, false, true};
	
	// insert them in the set
	options.insert(o1);
	options.insert(o2);
	options.insert(o3);
	
	// search for this one
	Option o = {"Hash", Option::Integer, 0, 0, 0};
	auto i = options.find(o);

	// display the found option
	std::cout << i->name << '\t'
		<< i->min << '\t'
		<< i->max << '\t'
		<< (i->type == Option::Boolean ? "bool" : "integer")
		<< std::endl;
}

And the output is:

Code: Select all

Hash	0	1	bool

while I would expect to find the Integer Hash option instead. Could anyone help me fix this code ?

Thank you

[lucasart]

I found my mistake: the operator< was incorrect. Should be:

Code: Select all

bool operator < (const Option& o) const
{ return type < o.type || name < o.name; }

And STL basically defines the equality predicate as

Code: Select all

(o1 == o2) <=> !(o1<o2 || o1>o2)

which perfectly explains why the first element of the std::set was a good find!

Anyway, stay tuned: I'll probably more stupid questions like that, as I try to figure out how to use C++ STL.

[mcostalba]

From what I understood googling around, it seems that std::set is more appropriate than std::vector for that.

Yes, but perhaps std::map it is even more, it avoids you to define compare operators and is more suitable (IMHO) for the "pick one option with a given key" scenario. I see std::set better in case you just need to find out if an element is in a set or not.

Code: Select all

#include <iostream>
#include <map>
#include <string>


struct Option {
   std::string name;
   enum Type {Boolean, Integer};
   Type type;
   int value, min, max;
};

typedef std::pair<std::string, Option::Type> Key;

std::map<Key, Option> options;

int main()
{
   // define a few options
   Option o1 = {"Hash", Option::Boolean, true, false, true};
   Option o2 = {"Hash", Option::Integer, 32, 1, 8192};
   Option o3 = {"Verbose", Option::Boolean, true, false, true};

   // insert them in the map
   options[std::make_pair(o1.name, o1.type)] = o1;
   options[std::make_pair(o2.name, o2.type)] = o2;
   options[std::make_pair(o3.name, o3.type)] = o3;

   // search for this one
   Key key = std::make_pair("Hash", Option::Integer);

   // here is our option
   auto i = options[key];

   // display the found option
   std::cout << i.name << '\t'
             << i.min << '\t'
             << i.max << '\t'
             << (i.type == Option::Boolean ? "bool" : "integer")
             << std::endl;
}

[lucasart]

From what I understood googling around, it seems that std::set is more appropriate than std::vector for that.

Yes, but perhaps std::map it is even more, it avoids you to define compare operators and is more suitable (IMHO) for the "pick one option with a given key" scenario. I see std::set better in case you just need to find out if an element is in a set or not.

Code: Select all

#include <iostream>
#include <map>
#include <string>


struct Option {
   std::string name;
   enum Type {Boolean, Integer};
   Type type;
   int value, min, max;
};

typedef std::pair<std::string, Option::Type> Key;

std::map<Key, Option> options;

int main()
{
   // define a few options
   Option o1 = {"Hash", Option::Boolean, true, false, true};
   Option o2 = {"Hash", Option::Integer, 32, 1, 8192};
   Option o3 = {"Verbose", Option::Boolean, true, false, true};

   // insert them in the map
   options[std::make_pair(o1.name, o1.type)] = o1;
   options[std::make_pair(o2.name, o2.type)] = o2;
   options[std::make_pair(o3.name, o3.type)] = o3;

   // search for this one
   Key key = std::make_pair("Hash", Option::Integer);

   // here is our option
   auto i = options[key];

   // display the found option
   std::cout << i.name << '\t'
             << i.min << '\t'
             << i.max << '\t'
             << (i.type == Option::Boolean ? "bool" : "integer")
             << std::endl;
}

Thank you Marco. Couple of questions
1/ what happens if I insert several options with the same key ? will I get them all in the map, or will the newly inserted one come and overwrite the one with the same key everytime ? I need to ensure uniqueness, so that's important.
2/ after

Code: Select all

auto i = options[key];

how do I know if the option corresponding to the key has been found or not ?

[mcostalba]

Hi Lucas,

1) std::map will overwrite so ensures uniqueness, std::multimap is used when more than one element with a given key is needed.

2) You don't. If a corrisponding option is not found a new one, corresponding to your key, is inserted. In case you need to check in advance, but it depends on the context. For instance in your case I guess options names are hardcoded in the program so don't find a given option is a bug. In this case an assert is perhaps more suitable:

Code: Select all

   // search for this one
   Key key = std::make_pair("Hash", Option::Integer);

   assert(options.count(key));

   // here is our option
   auto i = options[key]; 

[lucasart]

Hi Lucas,

1) std::map will overwrite so ensures uniqueness, std::multimap is used when more than one element with a given key is needed.

2) You don't. If a corrisponding option is not found a new one, corresponding to your key, is inserted. In case you need to check in advance, but it depends on the context. For instance in your case I guess options names are hardcoded in the program so don't find a given option is a bug. In this case an assert is perhaps more suitable:

Code: Select all

   // search for this one
   Key key = std::make_pair("Hash", Option::Integer);

   assert(options.count(key));

   // here is our option
   auto i = options[key]; 

Thanks! OK now I'm using an std::map, and I typically have to change engine options at runtime. Here is a sample code to modify the value of the option corresponding to key = (type, name)

Code: Select all

	Option::Key key = std::make_pair(type, name);
	auto it = options.find(key);
	
	if (it != options.end())
	{
		options[key].value = value;
	}

"it" is an std::map<Option::Key, Option>::iterator, right ? So *it should simply be an Option that I can modify. But when I try to use

Code: Select all

it->value = value;

the compiler insults me with an incomprehensible error message (as always in C++ with STL):

Code: Select all

error: ‘struct std::pair<const std::pair<Engine::Option::Type, std::basic_string<char> >, Engine::Option>’ has no member named ‘value’

The point is that

Code: Select all

options[key].value = value;

is inefficient, because we already have found the iterator (having done a tree-search). To modify the Option that we've already found, we have to look for it again (because options[key] does yet another tree-search) ? This can't be right... I'm sure there's a way to use my iterator directly.

[mar]

Shouldn't that be it->second = value instead?

[Sven]

I typically have to change engine options at runtime

I'd assume that typically you only change option values at runtime while the set of supported options itself stays constant. Your "struct Option" data structure combines a static, descriptive part specifying what your program supports, and a dynamic part containing a value per option, initially set to some default.

Sven