Changes coming - Input Welcome

As I work on the library with the editor in mind, I realize a lot of the current code was made so that Widgets can be created quickly and easily in code. I have made a lot of functions that take variable parameters, and make assumptions for default properties like texture and name. But why stop at texture and name? There are so many more properties that I set to default values. I can't possibly support them all. So why do I support any of them on creation? Here is a change I would like to make:

Currently the main class creating widgets is the Panel/Sheet/Window class. I have created a massive amount of functions, to support creating a widget with no supplied name or texture. It's a pain whenever I add a new widget to the list of creatable widgets. (4 creation methods, 1 actual creation method, 2 methods to retrieve widget) What I propose is that all widgets have a "default" state. The only thing I require are the dimensions. Every widget will have a list of supported child widgets, and it can only create widgets of this type. To create a widget, you would call something like

Button* b = dynamic_cast<Button*>(mSheet->createWidget(TYPE_BUTTON,Rect(0,0,20,10)));

And a Button will be created at pixel location 0,0, with a width of 20 pixels, and height of 10 pixels.

After creation of this Widget, you can tweak/set properties accordingly. The reason for this design change is that the editor will be doing exactly this. It will create a default widget, and then change properties as the user sets them in the example. I realize some people will generate their layouts in code, and this is still possible.

When the editor goes into development, a list of properties for each widget will be created. When it comes to serialization, I will make each widget have an additional constructor that takes all of these properties, and builds the widget from them. This will be more efficient than creating a default widget and building it from scratch.

Another reason I bring this up now is because I have implemented Borders and Resizing in real time. (tested with Window widget) The problem is that I have created 8 border widgets (4 corners, 4 edges) for every single widget, and peformance (load time) has worsened. Not every widget needs borders. Instead of having some fancy scheme that predicts and determines whether some widgets use borders or not (what I do currently is query whether the border texture exists.. 8 lookups per widget, still too costly), I would rather allow users to specify if borders are used. Widgets should be as small as necessary, while still supporting the various options a user may want to make use of. Another example of this is ScrollBars. Scrollpane's currently have 4 ScrollBar's (2 vertical 2 horizontal) but not everybody needs all of them used. (or any!) Instead of spending time and resources creating uneccessary objects, I would like to require the user to specify what they want.

Are there any major problems with this? I know that I have a couple users requests that I should be focusing on right this moment, but these changes are important to future implementation of Borders, ScrollPane's, Editor usage, etc.

If no posts are submitted, I have to assume there are no problems with this, and I will make the changes accordingly. Anything and Everything that requires a big change in API needs to be addressed in this upcoming version of QuickGUI. (if possible)

Just to restate, the main change is in the way widgets are created. Users will create a widget in its default state, and build it the way they want it.

Funny, I don't even think I should require dimensions, I can default those as well. In the editor, you will press a button or other to create a button, and it should not need to ask for dimensions, but instead create a button on screen, with some default dimensions.

Zini, I know that you don't plan on using the editor, but what do you think of the proposed changes? Your code would look more like CEGUI in this regard:


TextBox* tb = dynamic_cast<TextBox*>(mSheet->createWidget(TYPE_TEXTBOX));
tb->setSize(50,15);
tb->setPosition(25,75); // (or can setDimensions directly)
tb->setUseBorders(true);
tb->getText()->setFont("SomeFont.12");
tb->setGainFocusOnClick(false);
...


I have some comments on this, but I am really too sleepy now to write them down (past midnight here). I will post something tomorrow.

Its always once you start to fully flex your code do you realize all the things you missed in your design process. You might consider a more robust factory pattern than one you describe. If you make it flexible enough you should be able to plug in front ends that allow easy creation of widgets from configurations files, code, and serialized data.

Yah, I've been avoiding doing research on the Factory Pattern, mostly because of laziness. Are there any good examples of the Factory classes I should be looking at? Maybe the Ogre SceneManager factories?

I looked into Factories (Ogre SceneManager), and read the chapter on it in my design book, and I'm not sure how I can make use of the Factory pattern.

First, I'd have to convert the Widget::Type enumeration into a string value, and then I'd have to create some global type manager that can map strings into ints for faster comparison of widget types.. and then widgets would have to access this type manager to see if they are able to create a widget of type X..

Seems like a lot of work. Anybody think up a more elegant solution? Keep in mind that widgets have a predefined set of widgets they can create, and that the GUIManager is not a singleton. (There are no singletons) The goal would be to have Widgets able to create Custom Widgets, while maintaining a fast way to check Widget Type.

First, why do widgets create widgets? Shouldn't a Button widget at most create another button? It makes more sense to have single WidgetFactory that stores the proper information to map widget type

Second, why are you worrying about performance of string comparisons? Have you profiled your not yet written code to show that a few dozen string comparison in a std::map are causing a slow down?

The following is pretty powerful implementation of a factory pattern. Call it a "Maker" its based on a now lost article called "Industrial Strength Pluggable Object Factories". It supports customized return object types, object creational parameter types, and key types. It has the ability for you to customize how the object's key is extracted from the passed in parameters, and how its looked up in the registry, Yes the registry is global, but it is private (its a multimap so you can have widgets of the same name registered if you wish).

Note:
* It did take me many hours to write only 68 lines of highly template C++.
* This is also a good example of Policy based design
/ * Copyright (C) 2007 Robotics @ Maryland
* Copyright (C) 2007 Joseph Lisee
* All rights reserved.
*
* Author: Joseph Lisee
* File: packages/pattern/include/Maker.h
*
* Under the BSD License
*/

// STD Includes
#include <map>
#include <utility>
#include <iostream>
#include <cassert>

/*
This pattern is based on the "Industrial Strength Pluggable Factories"
article by Timothy R. Culp (with an added dose of policy based design)
*/


/** Default MakerLookup policy. Returns first maker which maps to the given key.
*/
struct DefaultMakerLookup
{
template<class MapType>
static typename MapType::mapped_type lookupMaker(MapType* registry,
typename MapType::key_type key)
{
return registry->find(key)->second;
}
};

/** Default ObjectMaker policy. Uses virtual makeObject method on maker objects.
*/
template<class Object, class Param>
struct DefaultObjectMaker
{
virtual ~DefaultObjectMaker() {};

/** Implemented by the subclasses of the Maker, creates the actual object */
virtual Object makeObject(Param param) = 0;

template<class Maker>
static typename Maker::ObjectType createObject(Maker* maker,
typename Maker::ParamType param)
{
return maker->makeObject(param);
}
};


/** Policy Based Object Creation Template
*
* This creates objects based on based in parameters use Maker objects
* registered to a specific key. It is templated based on the type of the
* object, parameter, and key. It also uses policies for extracting the key
* from the given parameters, finding the maker based from the multimap give
* the key, and finially creating the object using the found maker and the
* given parameter. @par
*
* Makers are registered in there constructor by calling the super class
* constrcutor with the desired key. Example:
* @code

struct StreamKeyExtractor
{
static std::string extractKey(std::iostream& param)
{
std::string key;
param >> key;
return key;
}
};

class Number
{
public:
virtual ~Number() {};
};

class Int : public Number
{
public:
Int(int _val) : value(_val) {};

int value;
};

typedef Maker<Number*, // The type of object created by the maker
std::iostream&, // The parameter used to create the object
std::string, // The type of key used to register makers
StreamKeyExtractor> // Gets the key from the paramters
NumberMaker;


class IntMaker : public NumberMaker
{
public:
IntMaker() : NumberMaker("Int") {};
virtual Number* makeObject(std::iostream& param)
{
int value;
param >> value;
return new Int(value);
}
private:
// Calls constructor to register the maker
static IntMaker registerThis;
};
// Needs to be in a cpp file to actually invoke the constructor
IntMaker IntMaker::registerThis;
};

* @endcode
*/

template<class Object, class Param, class Key,
class KeyExtract,
class MakerLookup = DefaultMakerLookup,
class ObjectCreate = DefaultObjectMaker<Object, Param> >

class Maker : public KeyExtract,
public MakerLookup,
public ObjectCreate
{
public:
typedef std::multimap<Key, Maker*> MakerMap;
typedef typename MakerMap::const_iterator MakerMapIter;

typedef Object ObjectType;
typedef Param ParamType;
typedef Key MappedType;

private:
Key m_key;

/** Returns the global registry for this maker
*
*/
static MakerMap* getRegistry()
{
// This line is run only once, avoids static initialization order issue
static MakerMap* reg = new MakerMap();
return reg;
}

public:

/** This constructor which registers the maker class in the registry
*
* @remarks
* When you subclass this class you should place a static instance
* of the class in its own definition. Then just make sure to call
* the super classes default constructor with the name of the class.
*
* @param key
* The object that identifies the class in the registry.
*/
Maker(Key key) :
m_key(key)
{
// Grab the registry (first use creates it on the heap)
MakerMap* registry = getRegistry();

// Register Self
registry->insert(make_pair(key, this));
}

/** Unregisters the Maker, and cleans up registry if necessary
*
* @remarks
* If this is the last class in the registry, it will delete the
* global registry.
*/
virtual ~Maker()
{
MakerMap* registry = getRegistry();

// Remove self from registry
registry->erase(m_key);

// Free up registry if we are the last in the registry
if (0 == registry->size())
delete registry;
}

/** Creates a new object based on the given parameters
*
* This uses the KeyExtract policy to pull the key from the parameters.
* Then it looks up the proper maker with the lookupMaker policy using that
* key. The default one uses the first maker which maps to the key. Then
* it creates the object by using the ObjectCreat Policy. The defualt
* policy just uses the virtual makeObject function.
*/
static Object newObject(Param param)
{
Key key(KeyExtract::extractKey(param));
Maker* maker = MakerLookup::lookupMaker(getRegistry(), key);

// TODO: raise an exception here
assert(maker && "Could not find object to match given key");
return ObjectCreate::createObject(maker, param);
}
};

Thank you for sharing this code. I'm still in the process of digesting it..

You really feel buttons should be able to creat buttons? (outside of a Widget::clone method?)

With the current design, you start with a Sheet, and from this Sheet, can create various widgets, for example a Window. This Window can then create a Panel, which can Create a Button, and a Label, ... etc.

The library isn't setup to support adding TextBoxes to a Menu, or a TitleBar to a Slider, etc. So I need to be able to use your maker pattern in combination with these constraints.

I hope to use your code along with a function such as:

Widget* Widget::createWidget(const Ogre::String& widgetType);

If the Widget cannot create the widget, NULL is returned.

Regarding string comparisons, I use them quite frequently. While they may not lower performance greatly, I'm trying to minimize anything that may effect performance, since the library is currently less performance friendly than I'd like. But this is not so important at the moment.. I want to get a working solution first, and then optimize second. So disregard my statement with string comparisons.

Looking at your code again. Thanks again for sharing.

Sorry for the inexperienced question, but does this code support creation of objects that:

1. Take no arguments (void)
2. Take X arguements, where X is the number of properties of a particular widget.

I suppose for 2. I can create a struct called "ButtonProperties" or something and pass that in for creation.


static Object newObject(Param param)
{
Key key(KeyExtract::extractKey(param));
Maker* maker = MakerLookup::lookupMaker(getRegistry(), key);

// TODO: raise an exception here
assert(maker && "Could not find object to match given key");
return ObjectCreate::createObject(maker, param);
}


The param is used to create the Object and to get the key? I don't understand how this works. The key would be "Button", and lets say I want no arguements. I'd probably have to make a StreamKeyExtractor class, or just modify the code not to include it.. ^_^.

Back to the question, say I want to create a "Button", how can I do this?

In your Number example, do you use something like "Int 5" as your params?

I think I answered my own question with "Int 5", that's probably how you do it.

So I know how to use your classes, but how can I easily distinguish between both constructors of a Widget?

Button();
Button(std::iostream& param);

Also, I like your iostream param method. I'm inexperienced with playing around with iostreams, but I imagine using an xml or script, and feeding that to the constructor, which can read the script and create the button from it. (cool!)

Glad you like the iostream stuff, really that's there just for testing you can use any type of parameters you want. The Params serve a dual purpose:
1) They identify which object you wish to create (This is what the KeyExtractor policy does)
2) They provide you with information to construct that object

So to continue about them and answer your questions:Can I creating Objects with no args
Yes the params can just specify the key. Example:class IntMaker : public NumberMaker
{
public:
IntMaker() : NumberMaker("Int") {};
virtual Number* makeObject(std::iostream& param)
{
// Ignore params, they just contained information to determine which
// object to create

// This could also be some kind of conditional which
// checks params for the desired information, and if not present creates
// the default object
return new Int();
}
private:
// Calls constructor to register the maker
static IntMaker registerThis;
};

Take X arguements, where X is the number of properties of a particular widget.
The Params variable is a templated type which can be anything you want. If the entire set of objects you wished to create takes the same arguments you can just create a little holder structure and make it the Params type. You could also use something like boost::tuple to generate these more easily on the fly.

About adapting to your interface:
If you want to customize the way your object is created (ie how the constructor is called you can do 2 things:
1) Implement the "makeObject" method of anyway you want. All it has to do is take the input params, and return the newly constructed object.
2) Create a custom ObjectMaker policy.



I recommend the first.

I am sure you have more questions so here is the test code for the pattern:
/*
* Copyright (C) 2007 Robotics at Maryland
* Copyright (C) 2007 Joseph Lisee
* All rights reserved.
*
* Author: Joseph Lisee
* File: packages/pattern/test/include/TestMaker.h
*
* Under the BSD License
*/

#ifndef RAM_PATTERN_TESTMAKER_H_08_10_2007
#define RAM_PATTERN_TESTMAKER_H_08_10_2007

// STD Includes
#include <sstream>
#include <iostream>
#include <string>

// A simple example set of class we wish to make dynamically
class Number
{
public:
virtual ~Number() {};
};

class Int : public Number
{
public:
Int(int _val) : value(_val) {};

int value;
};

class Double : public Number
{
public:
Double(double _val) : value(_val) {};

double value;
};

// Extracts the key from the iostream
struct StreamKeyExtractor
{
static std::string extractKey(std::iostream& param)
{
std::string key;
param >> key;
return key;
}
};

// A more verbose but simpler approach to using the pattern

typedef Maker<Number*, // The type of object created by the maker
std::iostream&, // The parameter used to create the object
std::string, // The type of key used to register makers
StreamKeyExtractor> // Gets the key from the paramters
NumberMaker;

class IntMaker : public NumberMaker
{
public:
IntMaker() : NumberMaker("Int") {};

virtual Number* makeObject(std::iostream& param)
{
int value;
param >> value;
return new Int(value);
}
private:
static IntMaker registerThis;
};

class DoubleMaker : public NumberMaker
{
public:
DoubleMaker() : NumberMaker("Double") {};

virtual Number* makeObject(std::iostream& param)
{
double value;
param >> value;
return new Double(value);
}
private:
static DoubleMaker registerThis;
};


// A More complex but less typing intensive method, this also lessens the
// amount a user of the system needs to know about its workings

template<class PrimType, class NumType>
struct NumberMakerTemplate : public NumberMaker
{
NumberMakerTemplate(std::string makerName) : NumberMaker(makerName) {};

virtual Number* makeObject(std::iostream& param)
{
PrimType number;
param >> number;
return new NumType(number);
}
};

class DoubleMakerVer2 : public NumberMakerTemplate<double, Double>
{
static DoubleMakerVer2 registerThis;
DoubleMakerVer2() : NumberMakerTemplate<double, Double>("DoubleVer2") {};
};

class IntMakerVer2 : public NumberMakerTemplate<int, Int>
{
static IntMakerVer2 registerThis;
IntMakerVer2() : NumberMakerTemplate<int, Int>("IntVer2") {};
};

#endif // RAM_PATTERN_TESTMAKER_H_08_10_2007


And the usage:/*
* Copyright (C) 2007 Robotics at Maryland
* Copyright (C) 2007 Joseph Lisee
* All rights reserved.
*
* Author: Joseph Lisee
* File: packages/pattern/test/src/TestMaker.cxx
*/

// Library Includes
#include <UnitTest++/UnitTest++.h>

// Test Includes
#include "pattern/include/Maker.h"
#include "pattern/test/include/TestMaker.h"

// Needed force creation of the static objects which register the makers
IntMaker IntMaker::registerThis;
DoubleMaker DoubleMaker::registerThis;

IntMakerVer2 IntMakerVer2::registerThis;
DoubleMakerVer2 DoubleMakerVer2::registerThis;


// See TestMaker.h for an example of how to create classes which use the maker
// pattern
TEST(basics)
{
std::stringstream test("Double 28.3 Int 10");

Number* newNum = NumberMaker::newObject(test);
Double* dbl = dynamic_cast<Double*>(newNum);
// Wrong object created
CHECK(dbl);
// Double not converted properly
CHECK_EQUAL(28.3, dbl->value);

newNum = NumberMaker::newObject(test);
Int* integer = dynamic_cast<Int*>(newNum);

// Wrong object created
CHECK(integer);
// Integer not converted properly
CHECK_EQUAL(10, integer->value);
}

TEST(basicsVer2)
{
std::stringstream test("DoubleVer2 28.3 IntVer2 10");

Number* newNum = NumberMaker::newObject(test);
Double* dbl = dynamic_cast<Double*>(newNum);
// Wrong object created
CHECK(dbl);
// Double not converted properly
CHECK_EQUAL(28.3, dbl->value);

newNum = NumberMaker::newObject(test);
Int* integer = dynamic_cast<Int*>(newNum);

// Wrong object created
CHECK(integer);
// Integer not converted properly
CHECK_EQUAL(10, integer->value);
}

int main()
{
return UnitTest::RunAllTests();
}

I started playing around with the code, and I realized I made a mistake. From the user, I require no arguments, however the underlieing Widget constructors remain unchanged.

Widget(const Ogre::String& instanceName, Type type, const Rect& pixelDimensions, Ogre::String textureName, QuadContainer* container, Widget* ParentWidget, GUIManager* gm);

The constructors for all the widgets are the same. I think this code would be a start to accomplish using the Make pattern:


struct WidgetParams
{
Ogre::String type; // Also the Key
Ogre::String instanceName;
Rect pixelDimensions;
Ogre::String textureName;
QuadContainer* container;
Widget* ParentWidget;
GUIManager* gm;
};

struct WidgetParamsKeyExtractor
{
static std::string extractKey(WidgetParams param)
{
return param.type;
}
};

typedef Maker<Widget*, // The type of object created by the maker
WidgetParams, // The parameter used to create the object
Ogre::String, // The type of key used to register makers
WidgetParamsKeyExtractor> // Gets the key from the paramters
WidgetMaker;


A thought just occurred to me now.

Lets say all Widgets had 2 constructors, with the same arguments:


Widget(const Ogre::String& instanceName, Type type, const Rect& pixelDimensions, Ogre::String textureName, QuadContainer* container, Widget* ParentWidget, GUIManager* gm);

Widget(QuadContainer* container, Widget* ParentWidget, GUIManager* gm, Ogre::StringVector properties);

Couldn't I simply typedef 2 function pointers, and have users add function pointers to a maintained list?

map<Ogre::String,plainConstructor> mPlainConstructors;
map<Ogre::String,advancedConstructor> mAdvancedConstructors;

Actually.. is it possible to have a function pointer to a constructor? Or is it somehow achievable by making a class? (new mPlainConstructor()(x,y,z,..); )

In reply to your original post, kungfoomasta :

It came to my mind too, that the create method's arguments are chosen rather arbitrary. You only have a subset of the widget's functionality, while the rest is set to default values. So you can as well clear up the create method and set everything to default instead. I am fine with that.

On the other hand I am strictly against the dynamic_cast approach. Excessive use of dynamic_cast often indicates a major design flaw. Besides that it is ugly, this design would demote some possible errors from compile time errors to runtime errors (which is a Very Bad Thing (TM)). Consider you accidentally try to create a button from a button. In your original design the compiler would complain. With the dynamic_cast variant it wouldn't. Instead the program would crash (there wouldn't even an exception been thrown by default, which makes things even worse).

I haven't read through all of the following posts, but I don't think a factory pattern is a good idea here either.
You only have a limited set of widgets, that is not expected to grow substantially in the future. Furthermore each widget can only create a limited set of widgets, so we have a fixed structure here, that does not require any dynamic behaviour.
With the factory pattern, you would sacrifice a substantial amount of static type checking and interface simplicity for an equally substantial amount of extensibility (which you won't use) and flexibility (which you won't use either).

If you are creating any set of objects from some sort of configuration setup the factor pattern can save you time. You can just feed it your stream of information and it will pop out the objects. This is the same functionality as a hard coded custom object loader but I think its more usable.

Now using this to replace all object creation methods is something up for debate, but I should point out it looks like he already has some of sort of factory method planned ("createWidget"). The machinery behind such a method is what the code I posted is about.

Thanks for the comments, guys. I think I should take a path somewhere in between.

I'll stick with the createButton/createLabel/etc instead of the createWidget route, since it defines the interface a bit more, and keeps the same scheme currently available. However, each function will take no parameters, and the output will be a Widget in its default state.

I have an idea for handling custom widgets, and I think I could add it in at a later date. It's not important for now, and won't cause any breaking changes to the API. In any case, you could currently create your own widget, and use the Widget::_addChild function to add your custom widget to another. (although I don't encourage use of this function a lot, since it allows bad combinations of widgets to be possible, for example, creating a TitleBar and making it a child of a List)

Very well, one of the cool things with that pattern is that it can seamlessly handle additions of new widgets types because of the magic static registration.

If some widgets really shouldn't be children of other widgets, do you have some sort of pre/post conditions check to ensure if an ignorant programmer (or a messed up configuration file) does so the library throws an error?

Don't get me wrong, I like your pattern, and I learned a good amount from it. I just think it is more than I need at the moment.

It actually made me think up a new technique, for better or worse. If all classes take the same types of arguments, you can make each widget have a create function, and use them in a map:


static Widget* Button::create(x, y, z);
static Widget* Label::create(x, y, z);
...
typedef Widget* (constructorPtr)(x,y,z);
map<Type,constructorPtr> mConstructors;
...
Button* b = dynamic_cast<Button*>(mConstructor[TYPE_BUTTON](x,y,z));


I haven't tested this, but assuming I can make those static methods and have pointers to them, this approach would be pretty awesome.

I should go ahead and make a list of supported children widgets for each widget, but I haven't done that yet. It would just be a map<Type,bool> to determine whether a given type can be added or not.

I should go ahead and make a list of supported children widgets for each widget, but I haven't done that yet. It would just be a map<Type,bool> to determine whether a given type can be added or not.

std::set<Type>

If the type->function works for you thats great. The trick is registering those functions without having to explicitly call a method at runtime like: "registerWidgetsCreateFunctions". Thats what all the static objects do in the Maker pattern.