• Advertisement
Sign in to follow this  

Generic pointer to template class (confusion)

This topic is 4259 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Okay, I've been trying to wrap my head around this one for a couple days, I'm hoping someone can point me in the right direction. I've got a class used for storing game settings (like cvars). The individual settings are instantiated from a single class, which contains code for storing and retrieving the settings. And, because different settings are in different formats (strings, integers, floats, etc.), it is a template class, and is capable of taking a value either in it's native data type or as a string and converting it to the appropriate data type for that template (using boost::lexical_cast). So, when I define a new setting, I can simply do something like: cvar<int> new_setting; new_setting = "100"; int i = new_setting.value; Now, my problem. In addition to the settings themselves, I also need to have a STL map which maps strings (containing the setting name) to the settings object itself. So, for example, a setting typed into the game console can be looked up by name and the object found, to read or write a value to: int i = settings["new_setting"]->value; The problem is the fact that the settings are template classes, so I can't exactly put a single type of pointer in the map and have it work for every instance. The only alternative I see is to use a base class that the settings derive from, and have the map use a base class pointer, but given how minimal the settings class is, I don't know what the base calss could possibly contain (there is no common code within the settings class that isn't using the template data type as a parameter). I suppose I could just use a void pointer and have something like: int i = reinterpret_cast<cvar<int> >(settings["new_setting"])->value; But that just looks terrible and opens up the potential for pointer problems. I hope this makes some sense. Ultimately, I'm trying to find a way for a central object to contain a pointer to a template class object, without knowing anything about the instances of the template class. So I can grab a pointer from one spot, and regardless of the data type of the object, call it's member functions to read and write data. Is this possible? Any ideas would be great.

Share this post


Link to post
Share on other sites
Advertisement
You might also consider not associating the type with the setting itself, but instead with the code that accesses it. Just store a map of string to string, and then offer a templated accessor:


template <typename T>
bool getSetting(T& result, const string& key) {
stringstream ss(theMap[key]);
return ss >> result;
}

int i;
if (getSetting(i, "new_setting")) {
// yay
}

Share this post


Link to post
Share on other sites
I'm just going to point out that a base class with no code is called an interface, and that's a very useful construct (Java, for example, uses it extensively and has it has given the concept own language construct - sorry if you knew this already). I'd personally go for a solution involving an interface like you sketched out in your post.
Otherwise, as Agony said, no need to reinvent the wheel ;-)

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
You might also consider not associating the type with the setting itself, but instead with the code that accesses it. Just store a map of string to string, and then offer a templated accessor:


template <typename T>
bool getSetting(T& result, const string& key) {
stringstream ss(theMap[key]);
return ss >> result;
}

int i;
if (getSetting(i, "new_setting")) {
// yay
}


Yeah, and looking at this solution it comes to mind, that you could even use it with your own classes - you only need to overload operator >> and that's it.
Nice.

Share this post


Link to post
Share on other sites
Wow, that was fast, already several great ideas. I'll have to look these over and see how they fit, thanks a lot guys!

Share this post


Link to post
Share on other sites
I've been looking over the ideas here, and I still have some concerns.

The Interface idea is the most attractive to me, because is seems fairly simple. However, again, I don't know what I could put into the base class that wouldn't turn it into a template. Every function within the settings class uses a template parameter. That would leave me with a base class containing absolutely nothing. Which the map happily accepts as a generic pointer, but which doesn't allow me to call the member functions of the settings which derive from the empty base class. Is there an alternative way to do this?

Performance is also a concern. Zahlman's idea of just storing each setting as a string and converting it to the appropriate data type when you need it using a template function is a novel idea, but also slow. The whole idea of originally craeting my settings class as a template was so that each data type could be stored natively, so that reading the value could be done without performance penalties. I plan to use these settings all over the place within the engine, so converting strings many many times per frame doesn't sound like a good idea.

It looks like boost::variant would pretty much do what I'm trying to accomplish with my settings class, but again it's complexity and body of features makes me wonder about run-time performance. Has anyone used them much? How much conversion does it do when working with the data? What penalties are there for reading the data back out as a particular type?

My other alternative is to forget the template idea, and just make the settings class contain multiple variables, one for each data type, and have it convert the written data to every format at once so that any one of them could be read back out.

cvar new_setting;
new_setting = "100";
int i = new_setting.integerValue;

Performance would be guaranteed, but this also just feels klunky, having to guess in advance what types will be needed and exposing them all at once. It might be my best bet, but I'm hoping there's still a way to make the template class idea work.

Share this post


Link to post
Share on other sites
Quote:
Original post by Nairou
Performance is also a concern. Zahlman's idea of just storing each setting as a string and converting it to the appropriate data type when you need it using a template function is a novel idea, but also slow. The whole idea of originally craeting my settings class as a template was so that each data type could be stored natively, so that reading the value could be done without performance penalties. I plan to use these settings all over the place within the engine, so converting strings many many times per frame doesn't sound like a good idea.

Well, yes, that doesn't sound like a good idea. But it is possible to solve this problem:
the hard way:
Try not to use Config directly. Read all values before game loop, store it in some struct(s).

the easy way:
Cache the values. Conversion from std::string to some other type will only happen when it is not cached (first request of the value or when the std::string changed).


I suppose you won't be trying to do it the hard way, so here's the sample of how would it work the easy way:

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

class ConfigValue
{
public:
ConfigValue(const std::string & val = "") : m_val(val), m_changed(true) {}
ConfigValue(const ConfigValue & val) : m_val(val.m_val), m_changed(true) {}
ConfigValue & operator = (const ConfigValue & val)
{
Set(val.m_val);
return *this;
}
ConfigValue & operator = (const std::string val)
{
Set(val);
return *this;
}
void Set (const std::string & val) { m_val = val; m_changed = true; }
// Auto-constructing from any type of value (except ConfigValue and std::string - those are already defined)
template< typename T >
ConfigValue (const T & val) : m_val(), m_changed(true)
{
std::ostringstream ss;
ss << val;
m_val = ss.str ();
}
// Auto-assigning from any type of value (except ConfigValue and std::string - those are already defined)
template< typename T >
ConfigValue & operator = (const T val)
{
std::ostringstream ss;
ss << val;
m_val = ss.str ();
m_changed = true;
return *this;
}
// Template functions that converts and caches the value
template< typename T >
T Get ()
{
static T cache;
if (m_changed)
{
std::istringstream ss (m_val);
ss >> cache;
m_changed = false;
}
return cache;
}
// No need to convert or cache anything, when the type is std::string
template<>
std::string Get ()
{
return m_val;
}
private:
std::string m_val;
bool m_changed;
};

int main ()
{
std::map<std::string, ConfigValue> Config;
Config["FullScreen"] = true;
Config["WindowTitle"] = "My cool game";
Config["WindowWidth"] = 800;
Config["WindowHeight"] = 600;
Config["FPS"] = 71.3f;
bool fs = Config["FullScreen"].Get<bool> ();
std::string wt = Config["WindowTitle"].Get<std::string> ();
int ww = Config["WindowWidth"].Get<int> ();
int wh = Config["WindowHeight"].Get<int> ();
float fps = Config["FPS"].Get<float> ();
std::cout << "Full screen: " << fs << std::endl;
std::cout << "Window title: " << wt << std::endl;
std::cout << "Window width: " << ww << std::endl;
std::cout << "Window height: " << wh << std::endl;
std::cout << "FPS: " << fps << std::endl;
return 0;
}



Note: the above is working example - i've tested it. ;)

Happy coding!

Share this post


Link to post
Share on other sites
Awesome, thank you very much for such a detailed and interesting reply! I hadn't thought of doing it that way, it's definitely given me a lot more to think about in how I might implement it. I'll have to spend some time going over this. Thanks!

Share this post


Link to post
Share on other sites
Quote:
Original post by Nairou
Performance is also a concern.

Is it?
How many times per frame are you going to access your settings?
10? 100? 100,000?
Unless it's at least the latter, performance is not a concern. At least not until you've implemented it, profiled it, and found it to slow.

Quote:

It looks like boost::variant would pretty much do what I'm trying to accomplish with my settings class, but again it's complexity and body of features makes me wonder about run-time performance. Has anyone used them much? How much conversion does it do when working with the data?

No conversion. It's essentially a nice type-safe wrapper around a void pointer. It doesn't "convert" your data to anything, it just ensures that you don't insert an element as one datatype, and then extract it as another. And most of that overhead is placed at compiletime, so there's almost no runtime overhead in using boost::variant or boost::any

Share this post


Link to post
Share on other sites
Are you sure of the line:
new_setting = "100";

because conversion from string to an int is inherently slow.. Unless "100" is the name for the setting rather than the value.

Unless cvar<T> inherits from a base class, each instantiation of cvar has no implicit relation with any other class of the template family.

Thus your problem degrades into a map that should contain different kinds of objects. I hate this because it squeezes the C++ syntax and makes it too confusing and unnatural.

Starting over, the design is at flaw. You should group configs which are the same type into a single map. In the first place, if you wanted to change an attribute, you must know its type.

Even yet, why use a map to store attributes (unless you worry about memory consumption, or if your data strutures are to be dynamic - this should not be an excuse if all states are given values upstart which defeats the purposes of memory saving). Consider instead using the conventional style of storing states (members) and using member pointers in conjunction for a more versatile design.


After a little thought, you can still use a config class template "cvar<T>" and use a map<string,cvar<T>> for each type instantiation. You should understand why this design is correct after observing your code:
int i = settings["new_setting"]->value;

you can notice that by using the settings["new_setting"] you know that its type is integer, which is not emphasized on the right side of the assignment.

Instead you could have used
int i = settings_int["new_setting"]->value;

Share this post


Link to post
Share on other sites
Quote:
Original post by Paulius Maruska

template< typename T >
T Get ()
{
static T cache;



Will that really generate a separate cache for each ConfigValue instance, for each type though? o_O

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Quote:
Original post by Paulius Maruska

template< typename T >
T Get ()
{
static T cache;



Will that really generate a separate cache for each ConfigValue instance, for each type though? o_O

Yes it will. As i said in the note bellow the code - i've tested it.
Although i probably need to mention that it worked on VC2005EE and i didn't test it on any other compiler.


EDIT: If you don't believe me, or you want to test this (on some other compilers), try this:


// change the templated Get method like this (add std::cerr line):
template< typename T >
T Get ()
{
static T cache;
if (m_changed)
{
std::cerr << " warning: value is not cached." << std::endl;
std::istringstream ss (m_val);
ss >> cache;
m_changed = false;
}
return cache;
}
// and now, change int main() function like this:
int main ()
{
std::map<std::string, ConfigValue> Config;
Config["FullScreen"] = true;
Config["WindowWidth"] = 800;
Config["WindowHeight"] = 600;
std::cerr << " notice: the warnings bellow is intended, because the values haven't been cached yet." << std::endl;
bool fs = Config["FullScreen"].Get<bool> ();
int ww = Config["WindowWidth"].Get<int> ();
int wh = Config["WindowHeight"].Get<int> ();
std::cout << "Full Screen: " << fs << std::endl;
std::cout << "Window width: " << ww << std::endl;
std::cout << "Window height: " << wh << std::endl;
std::cerr << " notice: if it doesn't work - we will see some warning's here." << std::endl;
int pixels = Config["WindowWidth"].Get<int> () * Config["WindowHeight"].Get<int> ();
std::cout << "Number of pixels: " << pixels << std::endl;
Config["FullScreen"] = !Config["FullScreen"].Get<bool>();
std::cerr << " notice: the warning bellow is intended, because the value changed." << std::endl;
std::cout << "Full Screen: " << Config["FullScreen"].Get<bool>() << std::endl;
return 0;
}



Now, when you run this - you should see exactly 4 warnings and three notices that explains what is going on.

[Edited by - Paulius Maruska on June 28, 2006 1:01:32 PM]

Share this post


Link to post
Share on other sites
Quote:
Original post by Spoonbender
Quote:
Original post by Nairou
Performance is also a concern.

Is it?
How many times per frame are you going to access your settings?
10? 100? 100,000?
Unless it's at least the latter, performance is not a concern. At least not until you've implemented it, profiled it, and found it to slow.


I disagree. Good design has a lot to do with performance, and is definitely not something to be saved until after implementation and profiling. Just because I can make my code really simple to write by storing everything as strings and then using STL to convert it to another data type every time I need it, doesn't mean the code will end up running efficiently. Profiling or not, it's bad design. That much is obvious, before writing a single line of code. I don't want to have to redesign and rewrite large portions of code after profiling just because I was too lazy to design it right the first time.

Quote:

Quote:

It looks like boost::variant would pretty much do what I'm trying to accomplish with my settings class, but again it's complexity and body of features makes me wonder about run-time performance. Has anyone used them much? How much conversion does it do when working with the data?

No conversion. It's essentially a nice type-safe wrapper around a void pointer. It doesn't "convert" your data to anything, it just ensures that you don't insert an element as one datatype, and then extract it as another. And most of that overhead is placed at compiletime, so there's almost no runtime overhead in using boost::variant or boost::any


I read the documentation for both boost::any and boost::variant, and what you describe sounds like boost::any. boost:variant lets you specify what data type you want to pull out of it and have it converted either automatically or with your own conversion functions. My question was about this functionality, the data conversion when the data is being pulled back out, as it sounds like a lot of overhead.

Share this post


Link to post
Share on other sites
Quote:
Original post by Paulius Maruska
Quote:
Original post by Zahlman
Quote:
Original post by Paulius Maruska

template< typename T >
T Get ()
{
static T cache;



Will that really generate a separate cache for each ConfigValue instance, for each type though? o_O

Yes it will. As i said in the note bellow the code - i've tested it.
Although i probably need to mention that it worked on VC2005EE and i didn't test it on any other compiler.

It seems to me, though, that if you did the following:
	int ww = Config["WindowWidth"].Get<int> ();
int wh = Config["WindowHeight"].Get<int> ();
ww = Config["WindowWidth"].Get<int> ();

That ww's final value would be equal to wh. In the main() that you posted, every single access was forced to go through the body of the "if (m_changed)" statement. But if you accessed "WindowWidth" again, then it hasn't changed, and so it returns that cached version, which I am mostly convinced is shared among all instances of class ConfigValue, which means it returns the "WindowHeight" cached version. And besides that, if in the same program you later tried the following:
	int ww = Config["WindowWidth"].Get<unsigned long> ();

that you'd still run into problems, because m_changed is false, since the value has already been retrieved (through a Get<int>()), but the unsigned long cache has never been initialized. Granted, accessing a value using two different types in the same program might then need to be considered inappropriate use, but I could imagine that occuring often enough, and causing some confusing bugs.

Personally, I'd suggest either having a predefined configuration class of some sort that reads the configuration file, and has methods to access each value, using the appropriate types, or just storing everything as a string and converting them when needed, since I agree with the others that doubt that the performance cost is going to matter much at all.

Share this post


Link to post
Share on other sites
Quote:
Original post by Nairou
Profiling or not, it's bad design.

What criteria do you use to determine that? Personally, I think it has a lot of design advantages. It is really easy to implement. Due to this, you will save time in development if it turns out to not be a performance bottle-neck. Also due to this, the likelihood of having strange bugs pop up because of a more complex configuration manager is reduced significantly, again potentially reducing development time.

When a method boasts quick implementation and minimal chance of bugs, I think it's safe to say that you can't just automatically call it bad design just because not as efficient as it could be.

Share this post


Link to post
Share on other sites
Quote:
Original post by Agony
Quote:
Original post by Paulius Maruska
Quote:
Original post by Zahlman
Quote:
Original post by Paulius Maruska

template< typename T >
T Get ()
{
static T cache;



Will that really generate a separate cache for each ConfigValue instance, for each type though? o_O

Yes it will. As i said in the note bellow the code - i've tested it.
Although i probably need to mention that it worked on VC2005EE and i didn't test it on any other compiler.

It seems to me, though, that if you did the following:
	int ww = Config["WindowWidth"].Get<int> ();
int wh = Config["WindowHeight"].Get<int> ();
ww = Config["WindowWidth"].Get<int> ();

That ww's final value would be equal to wh. In the main() that you posted, every single access was forced to go through the body of the "if (m_changed)" statement. But if you accessed "WindowWidth" again, then it hasn't changed, and so it returns that cached version, which I am mostly convinced is shared among all instances of class ConfigValue, which means it returns the "WindowHeight" cached version. And besides that, if in the same program you later tried the following:
	int ww = Config["WindowWidth"].Get<unsigned long> ();

that you'd still run into problems, because m_changed is false, since the value has already been retrieved (through a Get<int>()), but the unsigned long cache has never been initialized. Granted, accessing a value using two different types in the same program might then need to be considered inappropriate use, but I could imagine that occuring often enough, and causing some confusing bugs.

Personally, I'd suggest either having a predefined configuration class of some sort that reads the configuration file, and has methods to access each value, using the appropriate types, or just storing everything as a string and converting them when needed, since I agree with the others that doubt that the performance cost is going to matter much at all.


Damn, you are right... Now, that i'm looking to my own test - it doesn't work...
Anyway, you could also store the object for witch the value was cached, but then the conversion will happen a lot more often then it should...

Sorry for that.

Share this post


Link to post
Share on other sites
Well, okay, I agree it's subjective. For me, knowing that these settings are going to be heavily used throughout the engine, which means accessing them many times per frame (more and more and the engine is developed), performing unnecessary and slow data conversions within each frame just to make a small class quicker to write is, to me, not a good trade-off. I tend to put a lot of thought into design before coding, rather than coding everything as quickly as I can just so it works. In my mind, looking at the variety of options suggested so far in this thread, the one involving continual data conversion for every access didn't look like the best option for me to use (ignoring the fact that it was an innovative idea I hadn't previously thought of, and the fact that Paulius Maruska suggested an alternative that appears to speed up the same design idea).

Share this post


Link to post
Share on other sites
Quote:
Original post by Nairou
In my mind, looking at the variety of options suggested so far in this thread, the one involving continual data conversion for every access didn't look like the best option for me to use (ignoring the fact that it was an innovative idea I hadn't previously thought of, and the fact that Paulius Maruska suggested an alternative that appears to speed up the same design idea).


Except that my alternative doesn't realy work. :(

Share this post


Link to post
Share on other sites
Quote:
Original post by Paulius Maruska
Except that my alternative doesn't realy work. :(

It was a worthy try. I'm just sitting here advocating the lazy method. [smile]

Share this post


Link to post
Share on other sites
heh, and I may have to give in sooner or later. Or go with the other alternative, storing each data type separately in the class rather than using a template. I'm just stubbornly trying to find a way to make it work the way I want it. :)

Share this post


Link to post
Share on other sites
interface class:
two parts.
Part 1: virtual. Pure virtual methods that allow you to set/get int/double/string values.

Part 2: inline sugar. assign-from int/double/string and assign-to int/double/string. Calls the virtual set/get methods above.

implementation classes:
Inherits from the interface class above.

templated on the data type held.

Implements the virtual functions that let you set/get the value. Does conversions if required.

Side note:
You should use a non-mutable string with reference counting as the string you use to communicate from/to the get/set methods, so you can return strings from your class without doing copies.

Non-mutable reference-counted strings require no memory allocation to move them over multiple return steps. This is important if you want to treat strings like other primitive types.

Sketch of implementation:

template<typename T>
class property {
virtual void get(T*) const = 0;
virtual void set(T const*) = 0;
};

class property_interface:
public virtual property<int>,
public virtual property<double>,
public virtual property<my_string>
{
public:
// syntaxtic sugar:
operator int() { int retval; get(&retval); return retval; }
...
property_interface& operator=( int const& inp ) { set(&inp); return *this; };
...
private:
void operator=( property_interface const& ); // not implemented
};

// converts from U to T:
// possibly needs specialization if operator= doesn't do the right thing
template<typename T, typename U>
class property_convert:
public virtual property<T>,
public virtual property<U>
{
void get(T* t) const { U u; get(&u); *t = u; };
void set(T const* t) { U u = *t; set(&u); };
};

template<typename T>
class property_storage:
public virtual property<T>
{
T stored;
void get(T* t) const { *t = stored; }
void set(T const* t) { stored = *t; }
};

// type traits:
template<typename T>
struct other_types;

template<>
struct other_types<int> {
typedef double type1;
typedef my_string type2;
};

template<>
struct other_types<double> {
typedef int type1;
typedef my_string type2;
};

template<>
struct other_types<my_string> {
typedef int type1;
typedef double type2;
};

template<typename T>
class property_impl:
public virtual property_interface,
public virtual property_storage<T>,
public virtual property_convert< other_types<T>::type1, T >,
public virtual property_convert< other_types<T>::type2, T >
{
...
};


You store a map from string to property_interface references or pointers.

The properties can be read from and written to by using operator=. If you use the correct type, no conversion code is run. If you use another type, it is converted on the fly. Code to cache the conversion can be added at a later time.

[Edited by - NotAYakk on June 28, 2006 3:08:22 PM]

Share this post


Link to post
Share on other sites
Quote:
Original post by Nairou
Well, okay, I agree it's subjective. For me, knowing that these settings are going to be heavily used throughout the engine, which means accessing them many times per frame (more and more and the engine is developed), performing unnecessary and slow data conversions within each frame just to make a small class quicker to write is, to me, not a good trade-off. I tend to put a lot of thought into design before coding, rather than coding everything as quickly as I can just so it works. In my mind, looking at the variety of options suggested so far in this thread, the one involving continual data conversion for every access didn't look like the best option for me to use (ignoring the fact that it was an innovative idea I hadn't previously thought of, and the fact that Paulius Maruska suggested an alternative that appears to speed up the same design idea).


Some of my other posts here elaborate on this, but trying to anticipate what will really be a performance bottleneck is something that's difficult for even the most experienced developers to get right.

So, it's worth worrying about having a remedy for the future, but isn't necessarily worth worrying about right away. Easy examples in the case of the config stuff:

You wrap every config variable lookup. For debug, it's a class, and you use that class as the index into your map. Any attempt to use a raw string fails at compile time. For release, it's just a string. For now, it's easy, no-fuss, no-muss, and you've got working code.

Later on, you discover that it's actually a performance issue. You want a simple way to optimize it, and you don't care about cross-platform compatibility of any performance hacks. So, you turn on string pooling, rely on each string literal having a unique address, and use that as your map lookup. Gross, non-portable, but fast.

Or, you decide that you really need constant time performance. So, you write a (insert favorite hacky little scripting language here) script that scans your .cpp files for ConfigVar("foo"), and generates an enum containing all of your config variables. Then you switch your ConfigVar() macro to be
#define ConfigVar(varName) myEnumNamePrefix_ ## varName
and use it as a direct lookup into your pre-filled array of configuration variables.

Perhaps you decide that convenience and cleverness are important to you, so you write it as ConfigVarLookup("foo", myLocalFoo) so you can write template specialization code that can infer the type of the myLocalFoo variable, and look that up in a type-specific map, array or whatever.

Whatever you do, the important point is this: Your future-proofing consists of writing one class (that should be about 10 lines long) and two macros. Then, you're done, and everything works. You leave yourself open to any number of future optimizations, but you only pay the implementation cost when it has demonstrated itself to be necessary.

Of course, there's the fundamental question of why you need to repeatedly query a configuration database, instead of instantiating program-specific objects that embed those properties, but that's an entirely different subject - solving the right problem, rather than trying to prematurely address performance problems related to an artificial problem.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement