Jump to content
  • Advertisement
Sign in to follow this  
snooty

pointer, class, templates and design

This topic is 4323 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

Hi, I have a template class that looks like this:
enum FORMAT { F1, F2, F3 };

template<enum FORMAT f>
class C
{
public:
   void Foo() {...}
};


It's been working fine until when I want to wrap it up in a class that creates different objects according to a FORMAT flag:
FORMAT g_f;   //set at runtime before Wrap is constructed, and doesn't change once it is set

class Wrap
{
public:
   Wrap();
   ~Wrap();

   void Foo();

private:
   void *p;
};

Wrap::Wrap() {
   switch (g_f) {
   case F1:
      p = new C<F1>(); break;
   case F2:
      p = new C<F2>(); break;
   case F3:
      p = new C<F3>(); break;
   }
}

Wrap::~Wrap() {
   switch (g_f) {
   case F1:
      delete (C<F1> *)p; break;
   case F2:
      delete (C<F2> *)p; break;
   case F3:
      delete (C<F3> *)p; break;
   }
}

void Wrap::Foo() {
   switch (g_f) {
   case F1:
      ((C<F1> *)p)->Foo(); break;
   case F2:
      ((C<F2> *)p)->Foo(); break;
   case F3:
      ((C<F3> *)p)->Foo(); break;
   }
}


The problem is if C has 100 methods, I'll need to write the switch 100 times. Is there a way to work around it? Also, more fundamentally, is this good design to use the void pointer? I can't think of an alternative though... Please help.

Share this post


Link to post
Share on other sites
Advertisement
You could make Wrap also a template class:

[source lang=cpp]template <enum Format XX>
class Wrap
{
public:
Wrap();
~Wrap();

void Foo();

private:
C<XX> p;
};

(maybe my syntax is not correct, but you should get an idea)

Share this post


Link to post
Share on other sites
This sounds like a job for inheritance!

enum FORMAT { F1, F2, F3 };

// this is not a templated type!
class Base
{
public:
virtual ~Base (void)
{
}

virtual void foo (void) = 0;

static Base *Create (enum FORMAT g);
};

// the templated type which derives from a non-templated type
template <enum FORMAT f>
class C : public Base
{
// making these private enforces access via the base class pointer
private:
virtual ~C (void)
{
}

virtual void foo (void)
{
}

public:
// a helper function
static Base *Create (void)
{
return new C <f>;
}
};

// implementation of object creation based on template parameter
Base *Base::Create (enum FORMAT g)
{
Base
*p = 0;

switch (g)
{
case F1:
p = C<F1>::Create ();
break;
}

return p;
}

// some examples
void Bar (void)
{
const enum FORMAT g = F1;
Base *p = C<g>::Create ();
Base *q = Base::Create (F2);

// no need to use a switch here
p->foo ();
q->foo ();

// no need to use a switch here either
delete p;
delete q;
}

The only place a switch is needed is in the Base::Create function. Polymorphic behaviour is used to implement the remaining functions. Also, there's no need for a "void *" which is not type safe at all.

Skizz

Share this post


Link to post
Share on other sites
Fundamentally, you're doing what is called "type switching." Even though in this case, you aren't switching on a type per-se, you're switching on something that represents a type. This is brittle, as you've noticed, for many reasons.

This is the case even with Skizz's proposed solution, which is cleaner but does not solve the fundamental problem (it still involves a type-switch, although it is moving in the proper direction).

It seems to me that the problem you need solved is runtime creation of a type based on a value. For this, you want to implement the factory design pattern. Your Wrap class should really be a base class, not a containing class, as Skizz pointed out. Construction of "Wrap" instances is then gated on the value of g_f (or some other runtime determined value), so a factory or at the very least factory-like pattern seems appropriate.

The pattern is aimed at constructing subtypes of a certain base type based on runtime-determined "key" values (in this case, your FORMAT enum, but it could just as easily be a string). There are usually three core components to this pattern:

  • The "product" base class (the base class of the objects created)

  • The "producer" base class (the base class of objects designed to create "products")

  • The "factory" itself, which allows for key/producer relationships to be registered and which is ultimately used by client code to create new instances without resorting to type-switching.


Of course, you can implement this pattern in very sophisticated ways with templates and all sorts of nifty stuff to make a very abstract, generic and reusable factory implementation. There are plenty of good books and papers that cover that, however (see "Modern C++ Design," for example). I'll give you a very quick-and-dirty example here:

class Product
/* This class is a base class of all objects ultimately produced by the factory.
* You will generally end up with a 1:1 mapping between key values (remember
* the FORMAT values are your keys in this case) and subclasses of this type.
*/

{
/* whatever */
};

class Producer
/* This class is a base class of all subtypes used to create Product subtypes.
* As above, there is a 1:1 mapping between Product subtypes and Producer
* subtypes. Each Producer subtype implements a pure virtual method to create
* the appropriate Product subtype. This might seem unneccessary now, but it
* should become clear soon.
*/

{
public:
// Subtypes implement this method to produce concrete Product
// subclasses. A typical implementation would look like:
// ConcreteProduct* CreateProduct() { return new ConcreteProduct(); }
// (Note the use of "covariant return types" here).
Product* CreateProduct() = 0;
};

class Factory
/* This is the actual class used by client code.
* It allows Producers to be "registered" with it; this would be done
* at runtime early on, usually as soon as possible.
*
* Then, when a new instance is required, the factory's Create() method
* is called, taking the key for the desired type. A new pointer
* to the desired type is returned.
*/

{
public:
void RegisterProducer(FORMAT key,Producer *p)
{
mProducers.insert(std::make_pair(key,p));
}

Product* Create(FORMAT key)
{
return (mProducers[key]->Create());
}

private:
std::map< FORMAT, Producer* > mProducers;
};




Note that the usual caveats apply, I haven't actually compiled this code, it may contain syntax errors, and it definatley needs to do some error checking, but the logic should be sound.

Again, this is a basic implementation that has some issues -- for example, while the producer's can use covariant return types, that is hidden by the factory's Create method which transports the return type back to the base Product class. If Product's interface doesn't have appropriate virtual members, you might have to use a cast, which can defeat the purpose of the system. And there's no error checking at all. But those are exercises for the reader!

So, where's the advantage? First of all, there's no type-switching. In fact, there's very little code at all. Most of it is of the "write-once, reuse forever" flavor (although you do now have the added overhead of creating boilerplate producer classes for each product, but with some enginuity you can solve that as well). The second big advantage is that extending the system to support new products is trivial -- you implement an entirely new product type, and an entirely new producer (if you haven't solved that issue), and you register your producer with the factory in your client application. You don't have to touch the factory implementation itself, you don't have to remember to add to new type switches, et cetera. In some cases the producer registration can even be automated as well.

The end result is a system that is far more robust and generic than type-switching, and over the long term actually involves less developer elbow grease.

[Edited by - jpetrie on October 17, 2006 7:16:19 AM]

Share this post


Link to post
Share on other sites
jpetrie, that's a nice example of over-engineering the problem [smile], it doesn't solve anything that Skizz's solution doesn't and it doesn't solve the same problems any better. You still need to register an instantiation of your templated Producer class for each enumeration in FORMAT, which is no different to a case in the switch statement.

Here's some template-fu to automate the big switch statement in Skizz's Base::Create(). It's not pretty (or tested!), and assumes all enums in FORMAT start are consecutive between FORMAT_MIN and FORMAT_MAX, but it should work...


class Base
{

// Uses template recursion to in effect create
// your switch statement. It starts at FORMAT_MIN
// (which should be a value in FORMAT), and keeps
// going until it reaches the end condition below
template<enum F = FORMAT_MIN>
struct CreateHelper
{
static Base* Create(int f)
{
if(f==F) return C<(FORMAT)F>();
return CreateHelper<F+1>::Create(f);
}
};

// This is the end condition for the template recursion.
// If we get here and 'f' still doesn't match, return 0
// to indicate the value of 'f' is invalid.
template<>
struct CreateHelper<FORMAT_MAX>
{
static Base* Create(int)
{
if(f==FORMAT_MAX) return C<FORMAT_MAX>();
return 0;
}
};

public:

Base* Create(FORMAT f)
{
// This isn't essential, but it means we don't need to
// go through all of the template-generated if statements
// to discover that 'f' is invalid
if(f<FORMAT_MIN || f>FORMAT_MAX) return 0;

// Use our CreateHelper struct to generate the big
// switch statement instead of doing it by hand
return CreateHelper<>::Create(f);
}

};

Share this post


Link to post
Share on other sites
Quote:

jetrie, that's a nice example of over-engineering the problem


To be honest, I think its under-engineered. Largely because there are portions of it that are unneccessary (for example, templates can be used to automatically construct the Producer subclasses). However, that would have obfuscated the concept that I was trying to get across. I stated a number of times that the code I provided was only an example and shouldn't be consider suitable for production use.

Quote:

and it doesn't solve the same problems any better. You still need to register an instantiation of your templated Producer class for each enumeration in FORMAT,

The registration process could be automated as well (again, a "feature" I omitted for clarity of the idea) -- however, in practice, I've found that automating it is a brittle design choice, since most methods of doing so are subject to questionable static-initialization-order issues or couple the products (or producers) too tightly to the factory instance.

However, I disagree with "it doesn't solve the same problems any better," on a more important point: Skizz's solution requires modification of the base ("library") code in order to support new types. A generic factory does not; it requires modification of the client code to support new types (and since the client provides the types anyway, this is acceptable). Futhermore registration is a much "easier" process, in that it doesn't have to be structured in a specific way (whereas a switch statement might), and doesn't have to be done all at one site (whereas a switch statement does).

Also, your solution would break down as soon as the desired key type is, say, a string. It also requires all legal values of the key type to be known at compile time by the "base" code, meaning that extension is once again done in the "base" code, reducing the direct reusability of the solution.

[Edited by - jpetrie on October 18, 2006 7:14:59 AM]

Share this post


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

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!