Desc enhancement suggestion - input welcome

Currently I use a descriptor object to create widgets. This works really well, but as Widgets become more advanced, its hard to see what properties are specific to the widget. For example, a Panel Widget is derived from a ContainerWidget, derived from ComponentWidget, derived from Widget. The PanelDesc object is derived similarly. There is no real way for unfamiliar users to know which properties (members) are specific to Panels, Widgets, ContainersWidgets, etc.

My idea is to prepend all properties (members of these Desc classes) with the Widget Class name. For example:

Widget_Dimensions
Widget_Name
Widget_Resizable
...
ContainerWidget_SupportScrollBars
ContainerWidget_ScrollBarThickness
...

(this example isn't the greatest, since ComponentWidgets and Panels do not currently have specific properties associated with it )

So basically this allows use of intellisense to easily determine Widget specific properties (member variables). The only downside is that member variables are longer, so if you don't use intellisense, there might be a significantly larger amount of typing.

Any input on this idea?

Any input on this idea?
On first blush? "Eww"

That will break polymorphism. If a subclass wants to override a function, it has to use the same signature. So your PanelWidget will potentially have ContainerWidget_, ComponentWidget_ and Widget_ decorated function names.

Really that should just be a matter of documentation. If it's clear in your API docs then that is all that I need. The tooltip help in Eclipse's Java editor lets you know what class your member function/variable comes from no idea if Visual Studio has caught on to that yet.

well it would only affect naming of member variables, no member functions. For example here is the WidgetDesc def. (part of it):


class _QuickGUIExport WidgetDesc :
public BaseDesc
{
public:
WidgetDesc();

BrushFilterMode brushFilterMode;
bool consumeKeyboardEvents;
bool enabled;
Rect dimensions;
Ogre::String disabledSkinType;
bool dragable;
HorizontalAnchor horizontalAnchor;
float hoverTime;
Size maxSize;
Size minSize;
Ogre::String name;
bool resizable;
bool scrollable;
bool transparencyPicking;
VerticalAnchor verticalAnchor;
bool visible;
Ogre::String skinTypeName;
...


So to create a widget you would do this:

WidgetDesc d;
d.name = "myWidget";
d.resizable = false;
...

..createWidget(d);

So my proposal would rename these member variables, from "resizable" to "Widget_Resizable".

So in practice you would write:

...
d.Widget_Resizable = false;
...

The intellisense kicks in when you type the '.' after d. You should then have a list where you can see all Widget related properties, or other derived class properties. So If I have a new widget ie TabControl, you would know specific properties by trying to find member varaibles starting with "TabControl_"

I don't think this causes a problem for polymorphism or inheritance.

I think the idea to modify the coding style to placate a particular developer tool function is rather hilarious. But anyway, you are certainly right with this statement:


I don't think this causes a problem for polymorphism or inheritance.


But: What use is it? Why does a user of the QuickGUI library need to know which properties belong to which part of the widget hierarchy? IMHO the fact that, let's say the widget's background was defined in class y and not in class x (which y is derived from) is basically of no relevance for a user of class z, which is derived from class y.


On a slightly different topic: I haven't looked at the new version of QuickGUI yet, but from what I see these Desc classes are merely aggregations of public member variables with a constructor. Maybe you should make them structs instead. While that doesn't change anything, it expresses your intentions more clearly.

They are mainly structs, but they do carry some functions with them. This is a noob question, but can structs inherit from each other?

And you got the concept right, its just about renaming the member variables so that people can easily tell what properties are directly associated with the Widget they are trying to create. For example the TabControlDesc object, on top of all inheritted members, contains "selectedTab", "tabOverlap", "tabHeight", and "tabReordering" properties.

I wouldn't try out the newest version yet, until I've sorted out the OpenGL issue, otherwise it won't be useful to you. Plus I still need to get the Console widget working.

They are mainly structs, but they do carry some functions with them. This is a noob question, but can structs inherit from each other?


Yes. In C++ structs and classes are exactly the same, with the one exceptions, that class members are private by default and struct members are public by default (this only affects declarations before the first public, protected or private).

kungfoomasta: why not use a list of boost::any? Obviously you won't want to introduce boost as a dependency, so just use std::map with Ogre::Any:

typedef Ogre::Any any;
using Ogre::any_cast;
using std::string;
typedef std::map<string, any> description;

description desc;
desc["name"] = string("widget0");
desc["hoverTime"] = 12.0f;
desc["dimensions"] = Rect(0, 5, 0, 10);
// etc.

createWidget(desc);

createWidget(const description &desc)
{
description::iterator i, e = desc.end();

if ((i = desc.find("hoverTime")) != e)
{
setHoverTime(any_cast<float>(i.second));
}
}

etc.

There would only ever be a single description class. Whatever the widget cares about, it'll look for. You can create a little helper class instead of using the iterators directly. Here is what my similar code looks like, for example:

void foo(const parameters &params)
{
parameter p(params.end());

if (p = params.find("bar"))
setBar(p.value<bar>());
}



Yes. In C++ structs and classes are exactly the same, with the one exceptions, that class members are private by default and struct members are public by default (this only affects declarations before the first public, protected or private).
Structs also inherit publicly by default while classes inherit privately by default.

Wow, that's pretty slick! But that requires users to know there is a "name", "hoverTime", and "dimensions" property right? While the desc class is mostly a struct of strings, bools, ints, etc. there is a factory function for it, as well as a function outlining how to serialize it with the serial system I have in place. Its actually really cool since all my Widget properties are stored in this one struct-like class, so serialization is just a matter of writing and loading the desc objects from disk, and using them to initialize widgets.

Now this is getting pretty interesting.

@kungfoomasta: Serialisation of nullsquared's desc type should be even easier. Just iterate over the map and store an additional type information with each entry. With your version you would need to write two functions for each struct. Sounds like a lot more work, especially if you want your widget set to be extensible.

Or, if you want to get fancy, you could create a system-wide registry for names which stores the type. Instead of a simple map you would then use a wrapped map, which automatically checks the type (by looking the name up in the registry) and throw an exception, if it does not match. The same principle could be used to implement serialization, so you would not need to store the type-information with the serialized data.

@nullsquared: Your concept looks much cleaner, but I see one drawback. With kungfoomasta's structs the attributes are bound to the various levels of the inheritance hierarchy. That means you can easily copy e.g. the basic widget class' attributes to a new desc (with just a little bit of casting) Depending on how the inheritance hierarchy looks, this could be pretty useful for creating series of widgets (maybe of different types) with the same basic properties.

On the other hand, if we want to elaborate more an the wrapped map idea, we could enhance the registry to store not only the name, but the widget class the attribute belongs to (as a side-effect this would allow a better testing for reading in bogus widget data through the serialization system). And then add a function to the wrapped map to copy attributes over for specific levels of the inheritance hierarchy.




Yes. In C++ structs and classes are exactly the same, with the one exceptions, that class members are private by default and struct members are public by default (this only affects declarations before the first public, protected or private).
Structs also inherit publicly by default while classes inherit privately by default.


Wow! Just learned something new about C++. That doesn't happen often these days. Not terrible useful, but thanks anyway.

Wow, that's pretty slick! But that requires users to know there is a "name", "hoverTime", and "dimensions" property right? While the desc class is mostly a struct of strings, bools, ints, etc. there is a factory function for it, as well as a function outlining how to serialize it with the serial system I have in place. Its actually really cool since all my Widget properties are stored in this one struct-like class, so serialization is just a matter of writing and loading the desc objects from disk, and using them to initialize widgets.
Well, in general, yes, the user will need to manually know about each parameter. Then again, they don't really gain much by having them hard-coded other than intellisense or something, and a nice organized list of the different description attributes for each type of widget can be a lot more useful.

As for serialization, that's a different issue. Assuming each value is serializable to a string, you could probably do something like this:

#define OUT_ANY(os, rhs, T) do { if (rhs.type() == typeid(T)) os << boost::any_cast<T>(rhs); } while (false)

std::ostream &operator<<(std::ostream &os, const boost::any &rhs)
{
OUT_ANY(os, rhs, float);
OUT_ANY(os, rhs, double);
OUT_ANY(os, rhs, unsigned);
OUT_ANY(os, rhs, signed);
OUT_ANY(os, rhs, std::string);
OUT_ANY(os, rhs, bool);
// etc.
// custom classes may need custom operator<<()'s

return os;
}

Obviously that uses a string stream, but you can adapt it to your own serialization routines. It requires RTTI, though - and will not be crazy fast. But then again, writing/reading to disk will not be crazy fast to begin with.

And for deserialization, do you know what each property's type should be? Assuming you read in 'size', do you expect 4 floats? In which case, that shouldn't be too hard - just read in the values, do a lexical_cast (again, because of boost, make your own lexical_cast using std::stringstream) to a float or whatever you expect, and then feed that to an any(), and stick it in the description map.

Now this is getting pretty interesting.

@kungfoomasta: Serialisation of nullsquared's desc type should be even easier. Just iterate over the map and store an additional type information with each entry. With your version you would need to write two functions for each struct. Sounds like a lot more work, especially if you want your widget set to be extensible.

Or, if you want to get fancy, you could create a system-wide registry for names which stores the type. Instead of a simple map you would then use a wrapped map, which automatically checks the type (by looking the name up in the registry) and throw an exception, if it does not match. The same principle could be used to implement serialization, so you would not need to store the type-information with the serialized data.

I don't think you need to necessarily store any type information as long as the serialization knows what to expect (see my post above).

@nullsquared: Your concept looks much cleaner, but I see one drawback. With kungfoomasta's structs the attributes are bound to the various levels of the inheritance hierarchy. That means you can easily copy e.g. the basic widget class' attributes to a new desc (with just a little bit of casting) Depending on how the inheritance hierarchy looks, this could be pretty useful for creating series of widgets (maybe of different types) with the same basic properties.

That's even easier, no casting:

description base;
base["size"] = size;
base["parent"] = parent;
base["background"] = background;
base["font"] = font;

description d = base; // copy over the base details
// add more stuff
d["title"] = title;
d["image"] = image;




Wow! Just learned something new about C++. That doesn't happen often these days. Not terrible useful, but thanks anyway.
It's not useful, but it's one of those C++ 'gotcha!' moments when you change a struct to a class or vice versa and all of a sudden you get weird inheritance errors.

I don't think you need to necessarily store any type information as long as the serialization knows what to expect (see my post above).


It's not just serialisation. If I am not mistaken, these structs/maps are used to construct widgets manually too.And if we use Ogre::Any instead of structs we lose the type-safety at this point. Bad map-entries would show up inside the widget's constructor anyway, but maybe it would be advantageous to catch these kinds of problems earlier. Not really sure about this point. Haven't thought it through yet.

Regarding the base-class stuff. That is not what I meant. With your version the user has to manually specify the attributes and if a new QuickGUI versions adds anything to the base class it would be lost (and probably pretty hard to track down).
To be honest I can't come up with a concrete usage scenario for this feature. I just have the feeling, that it could eventually become useful.

And for deserialization, do you know what each property's type should be? Assuming you read in 'size', do you expect 4 floats? In which case, that shouldn't be too hard - just read in the values, do a lexical_cast (again, because of boost, make your own lexical_cast using std::stringstream) to a float or whatever you expect, and then feed that to an any(), and stick it in the description map.


That would obviously work, but it would require each widget to implement its own deserialization function. If the number of widgets isn't too large and the focus is not too heavy on custom widgets, your approach looks more efficient. But if we get a large number of widgets or the user is implementing lots of custom widgets mine looks better.

I don't see why the any's should be tied to any particular type of widget. If you want run-time widget construction (which is what I am getting from your post), then you can use factories. Register a widget factory with a certain widget type, and then just go:

description d;
d["type"] = string("scrollbar");
d[...] = ...;

createWidget(const description &desc)
{
widget *w = NULL;
parameter p(desc.end());
if (p = desc.find("type"))
{
string type = p.value<string>();
// do we have a factory for this widget type?
if (factories.find(type) != factories.end()) // yes, we do
w = factories[type]->create(desc);
else // no factory for it
log("no widget factory for type \"" + type + "\"");
}
else
log("no widget type");
return w;
}

etc.


widget window
{
size 10 10
position 15 25
title "the awesome scrollbar"
background "back.png"
...
}


// when it's read in, it'd be
d["type"] = "window";
d["size"] = vec2(10, 10);
d["position"] = vec2(15, 25);
d["title"] = title;
...

Obviously, it'll all be picked up at run-time. Indeed there are many gotchas and misses to this idea (for example, how would you know that size is a vec2 unless you're manually checking for 'size', etc.), so I'm just throwing out my $0.02.

Looks like we can not reach an agreement about the optimal design here. Anyway, we are pretty far from kungfoomasta's original design and he has been silent for a while now. Hope we didn't scare him off

To sum up the advantages/disadvantages of the various designs:

kungfoomasta's:

1. plain struct with a constructor; must be sub-classed for each widget[/*:m]
   
2. no invalid description can exist (except for bad values)[/*:m]
   
3. serialisation and de-serialisation must be implemented on a per-widget basis[/*:m]
   
4. property-names can be looked up via various IDE-tools without consulting the documentation[/*:m]
   
5. default-values for properties can be specified in the struct's constructor[/*:m][/list:u]
   
   
   nullsquared's:
   
   1. plain map (name-value pairs with flexible value-type)[/*:m]
      
   2. invalid descriptions can exist and must be checked for in the widget-implementation[/*:m]
      
   3. serialisation does not need to be implemented on a per-widget basis[/*:m]
      
   4. de-serialisation must be implemented on a per-widget basis[/*:m]
      
   5. default-values for properties can/must be specified in the widget's constructor[/*:m][/list:u]
      
      
      mine:
      
      
      1. intelligent map, that validates the types for each name[/*:m]
         
      2. needs an additional registry-class[/*:m]
         
      3. no invalid descriptions can exist (except for bad values)[/*:m]
         
      4. widget types must store their name-type information in a registry[/*:m]
         
      5. neither serialisation nor de-serialisation need to be implemented on a per-widget basis[/*:m]
         
      6. default-values for properties can be stored in the registry and can even be changed globally by the user[/*:m][/list:u]
         
         (I excluded the part about matching types to widgets, because it is a purely optional point of this design)
         
         About the default values: This is a point that wasn't brought up yet and it is pretty much independent from the design-choices listed above.
         With an increasing number of widget properties, IMHO it would make sense to offer reasonable default values for properties, so the user doesn't have to set all of them, when creating widgets in his code.
         This would help too when later versions of QuickGUI would add another property to a widget, which would invalidate user code for older QuickGUI-versions, unless default values are available.

nullsquared's:

1. invalid descriptions can exist and must be checked for in the widget-implementation[/*:m][/list:u]
   

Invalid descriptions can be easily ignored. Unless you're talking about wrongly-typed descriptions, which will need be taken care of via a try-catch statement.

1. de-serialisation must be implemented on a per-widget basis[/*:m]
   
2. default-values for properties can/must be specified in the widget's constructor[/*:m][/list:u]
   

de-serialization doesn't need to be per-widget. Various keywords like "size' or "titlebar" can have "hooks" that read them:

readSize(description &d, std::string line)
{
...
d["size"] = size;
}

commandDictionary["size"] = &readSize;

As for the widget type:

widget TitleBar
{
...
}

It'd be as simple as reading in whatever comes after 'widget' and putting it as the "type" in the description.

Default values can be done during description reading. Like:

std::string name = "unnamedWidget";
if (p = params.find("name"))
name = p.value<string>();


But, yeah, we both have very involved ideas and it'd be best to hear from kungfoomasta

I'm still here! haha. Just really busy lately. I don't want to post without fully digesting the ideas presented, I kind of glanced over them briefly. I will re-read and post a better reply tomorrow.

@nullsquared: I am fully aware of all the stuff you wrote in year last post. It seems we have very different ideas about handling error situations. While your methods would catch all errors eventually, they would be caught pretty late.

Catching errors as early as possible looks strongly preferable to me (for the sake of robustness, maintainability and so on ...). Especially data read in from a file should be put under a sanity check and IMHO this should happen at the time when the file is read in.

P.S.: Your commandDictionary looks remarkably similar to the name/type-registry, that I proposed.

I'm not sure I fully understand how the wrapped map class works. How can I ensure type safety?

Given this example:

desc["dimensions"] = Rect(0, 5, 0, 10);

How can I create a registry, or mapping, that says that "the property with name "dimensions" must be of type Rect"?

While its true I have to code in serialization for each Desc type, its actually very easy with the serialization I have in place. There is a SerialBase class, which outlines the interface for SerialWriter and SerialReader. Using this design I only need 1 function for both serialization and deserialization, here are some member functions of SerialBase:

void IO(const Ogre::String& propertyName, Rect* member) = 0;
void IO(const Ogre::String& propertyName, Ogre::String* member) = 0;
...

All serializable classes implement the following:

void Serialize(SerialBase* b);

Here is what the widgetDesc::serialize function looks like:


void WidgetDesc::serialize(SerialBase* b)
{
b->IO("BrushFilterMode",&brushFilterMode);
b->IO("ConsumeKeyboardEvents",&consumeKeyboardEvents);
b->IO("Enabled",&enabled);
b->IO("Dimensions",&dimensions);
b->IO("DisabledSkinType",&disabledSkinType);
b->IO("Dragable",&dragable);
b->IO("HorizontalAnchor",&horizontalAnchor);
b->IO("HoverTime",&hoverTime);
b->IO("MaxSize",&maxSize);
b->IO("MinSize",&minSize);
b->IO("Resizable",&resizable);
b->IO("Scrollable",&scrollable);
b->IO("TransparencyPicking",&transparencyPicking);
b->IO("VerticalAnchor",&verticalAnchor);
b->IO("Visible",&visible);
b->IO("SkinType",&skinTypeName);
}


This can only work if all data types can be converted to string. This is why I maintain a StringConverter class, like Ogre does. This also means you can convert a lot of my enumerated types into string form.

For derived desc types, I just call the parent's serialize function, and add on lines specific to that desc object. In the design above the serialize/deserailize functions will always iterate over properties in the exact same order, but since I use property names, order doesn't really matter.

And just a high level view of my serialization system: the SerialWriter populates ScriptDefinition classes, and the SerialReader extracts data from ScriptDefinition classes. The ScriptWriter class writes ScriptDefinitions to file or string, and the ScriptReader class reads from file or string to create ScriptDefinitions.

How can I create a registry, or mapping, that says that "the property with name "dimensions" must be of type Rect"?


The registry would simply be another map, which contains a name/type-information pair.
When trying to access an entry in the desc class, it would look up the name in the registry.

Regarding the desc class itself: The interface would be more explicit, if you add for each supported type a get/set function.
But if you want to keep the [] syntax, you would need to return a handle (small helper class) from operator[], which overloads the operator=. This operator= would need to check the type and then store the assigned value in the desc class instance.

Edit: Both nullsquared and me got carried away somewhat with our designs. While both have a certain elegance and scale better with a large number of widget types, they are probably a bit of an overkill for the problem at hand. If your model is already in place and fully working, another re-implementation is hard to justify IMHO.

Thanks for the input all.

If I was starting from scratch I would probably adopt one or a combination of ideas posted here, but to write this in now would take a lot of work. I do not think the current design is that bad, but if you dislike it I'd like to know why.

Since there is much to work on, I will proceed with the quick modification of applying the Widget Class name in front of specific properties, to allow IDE aided organization. (intellisense)

Thanks for the input all.

If I was starting from scratch I would probably adopt one or a combination of ideas posted here, but to write this in now would take a lot of work. I do not think the current design is that bad, but if you dislike it I'd like to know why.

Since there is much to work on, I will proceed with the quick modification of applying the Widget Class name in front of specific properties, to allow IDE aided organization. (intellisense)
I don't necessarily dislike your way of doing it, I just got carried away debating dynamic name-value containers