tags:

views:

166

answers:

4

I'm doing something that seems like it could be improved, but I don't have sufficient skill to improve it. Can you help?

Given:

vector<Base*> stuff;
const vector<MetaData>& metaDataContainer = Config.getMetaData();

for(vector<MetaData>::const_iterator i = metaDataContainer.begin(), end = metaDataContainer.end(); i != end; ++i)
{
  Base* pCurrent = buildDerivedType(*i);
  stuff.push_back(pCurrent);
}

Base* buildDerivedType(MetaData meta)
{
  Base* pRetval = NULL;

  switch(meta)
  {
    case MetaData::A:
    pRetval = new Alpha();
    break;

    case MetaData::B:
    pRetval = new Beta();
    break;

    //so on so forth
  };
  return pRetval;
}

I feel like the switch statement is bad because at compile time all the enum values are known, so theoretically we already know what types need to go into vector stuff. But we do that at runtime.

Short of writing a code generator for this, is there a better way?

+3  A: 

Not really. However, you can abstract away much of the boilerplate with a factory type and use boost::ptr_vector or a container of smart pointers to manage the resources in a smart way. (See comments about the choice between smart container vs dumb container of smart pointers.)

Roger Pate
+3  A: 

Well, you can preallocate a mapping meta -> function which creates the needed derived type.

typedef Base* ((*DerivedTypeCreator)());
map <MetaData, DerivedTypeCreator> derivedTypeBuilders;

// ...
derivedTypeBuilders[MetaData::A] = &CreateAlpha;
derivedTypeBuilders[MetaData::B] = &CreateBeta;

Base* CreateAlpha()
{
    return new Alpha();
}

Base* CreateBeta()
{
    return new Beta();
}

// ...
for(vector<MetaData>::const_iterator i = metaDataContainer.begin(),
        end = metaDataContainer.end();
    i != end; ++i)
{
    stuff.push_back((*(derivedTypeBuilders[*i]))());
}

etc.

Please don't forget to consider the case when the value of meta is not in the map!

For the specific case if your Metadata values are small numbers, you can even eliminate runtime overhead:

DerivedTypeCreator[] derivedTypeBuilders = {
    &CreateAlpha, // Metadata::A == 0
    &CreateBeta   // Metadata::B == 1
};

// ...
stuff.push_back((*(derivedTypeBuilders[(int)*i]))());

-- but this approach is perhaps too low-level and error-prone. (Hope such initialization is allowed by C++ syntax.)

Vlad
A: 

You are asking about the Factory Pattern. You can google for that. To put as much as possible into compile-time (w.r.t. run-time) you probably want to add the search terms using templates.

But, since you ''vector of metadata'' contains the information at runtime, you will need something dynamic. The enum is not that bad an idea -- look here, for example: http://www.daniweb.com/forums/thread18399.html. Note also the bit at the end with typedef InstantiatorMap::value_type InstantiatorMapEntryType. A mapping of ''enums'' to ''member function pointers'' as factories is used.

towi
Ah, and you might want to update the title of you question... if possible.
towi
+2  A: 
template<class T> Base* Creator() { return new T; }
typedef Base* (*CreateType)();
typedef std::pair<const MetaData, CreateType> cpair;

cpair mapper [] =
{
    cpair(MetaA, Creator<A>),
    cpair(MetaB, Creator<B>),
};

std::map<MetaData, CreateType> typemap(&mapper[0], &mapper[sizeof(mapper)]);

void foo(MetaData m)
{
    Base* a = typemap[m]();
}
Gene Bushuyev
Nice usage of map constructor.
Vlad