views:

2672

answers:

10

I have an application that has several objects (about 50 so far, but growing). There is only one instance of each of these objects in the app and these instances get shared among components.

What I've done is derive all of the objects from a base BrokeredObject class:

class BrokeredObject
{
  virtual int GetInterfaceId() = 0;
};

And each object type returns a unique ID. These IDs are maintained in a header file.

I then have an ObjectBroker "factory". When someone needs an object, then call GetObjectByID(). The boker looks in an STL list to see if the object already exists, if it does, it returns it. If not, it creates it, puts it in the list and returns it. All well and good.

BrokeredObject *GetObjectByID(int id)
{
  BrokeredObject *pObject;
  ObjectMap::iterator = m_objectList.find(id);
  // etc.
  if(found) return pObject;

  // not found, so create
  switch(id)
  {
    case 0: pObject = new TypeA; break;
    case 1: pObject = new TypeB; break;
    // etc.
    // I loathe this list
  }
  // add it to the list
  return pObject;
}

What I find painful is maintaining this list of IDs and having to have each class implement it. I have at least made my consumer's lives slightly easier by having each type hold info about it's own ID like this:

class TypeA : public BrokeredObject
{
  static int get_InterfaceID() { return IID_TYPEA; }
  int GetInterfaceID() { return get_InterfaceID(); }
};

So I can get an object like this:

GetObjectByID(TypeA::get_InterfaceID());

Intead of having to actually know what the ID mapping is but I still am not thrilled with the maintenance and the potential for errors. It seems that if I know the type, why should I also have to know the ID?

What I long for is something like this in C#:

BrokeredObject GetOrCreateObject<T>() where T : BrokeredObject
{
  return new T();
}

Where the ObjectBroker would create the object based on the type passed in.

Has C# spoiled me and it's just a fact of life that C++ can't do this or is there a way to achieve this that I'm not seeing?

+3  A: 

Instead of GetInterfaceId() in the BrokeredObject base class, you could define that pure virtual method:

virtual BrokeredObject& GetInstance()=0;

And in the derived classes you'll return from that method the instance of the particular derived class, if it's already created, if not, you'll first create it and then return it.

Boyan
And what object would you use to invoke the method? There are no such things as "virtual static" methods (what would it mean).
erikkallen
Every derived class would be a singleton. Those are the objects I'll use to invoke the method.
Boyan
+1  A: 

If you have RTTI enabled, you can get the class name using typeid.

One question, why are you using a factory rather than using a singleton pattern for each class?


Edit: OK, so you don't want to be locked into a singleton; no problem. The wonderful thing about C++ is it gives you so much flexibility. You could have a GetSharedInstance() member function that returns a static instance of the class, but leave the constructor public so that you can still create other instances.

Mark Ransom
I'm running under Windows CE without RTTI.I'm not using the singleton pattern because there may be some cases where an object isn't necessarily a singleton and I don't want to rigidly enforce it at this point and end up having to change it later.
ctacke
+8  A: 

Yes, there is a way. A pretty simple even in C++ to what that C# code does (without checking for inheritance though):

template<typename T>
BrokeredObject * GetOrCreateObject() {
  return new T();
}

This will work and do the same as the C# code. It is also type-safe: If the type you pass is not inherited from BrokeredObject (or isn't that type itself), then the compiler moans at the return statement. It will however always return a new object.

Singleton

As another guy suggested (credits to him), this all looks very much like a fine case for the singleton pattern. Just do TypeA::getInstance() to get the one and single instance stored in a static variable of that class. I suppose that would be far easier than the above way, without the need for IDs to solve it (i previously showed a way using templates to store IDs in this answer, but i found it effectively is just what a singleton is).

I've read that you will leave the chance open to have multiple instances of the classes. One way to do that is to have a Mingleton (i made up that word :))

enum MingletonKind {
    SINGLETON,
    MULTITON
};

// Singleton
template<typename D, MingletonKind>
struct Mingleton {
    static boost::shared_ptr<D> getOrCreate() {
        static D d;
        return boost::shared_ptr<D>(&d, NoopDel());
    }

    struct NoopDel {
        void operator()(D const*) const { /* do nothing */ }
    };
};

// Multiton
template<typename D>
struct Mingleton<D, MULTITON> {
    static boost::shared_ptr<D> getOrCreate() {
        return boost::shared_ptr<D>(new D);
    }
};

class ImASingle : public Mingleton<ImASingle, SINGLETON> {
public:
    void testCall() { }
    // Indeed, we have to have a private constructor to prevent
    // others to create instances of us.
private:
    ImASingle() { /* ... */ }
    friend class Mingleton<ImASingle, SINGLETON>;
};

class ImAMulti : public Mingleton<ImAMulti, MULTITON> {
public:
    void testCall() { }
    // ...
};

int main() {
    // both do what we expect.
    ImAMulti::getOrCreate()->testCall();
    ImASingle::getOrCreate()->testCall();
}

Now, you just use SomeClass::getOrCreate() and it cares about the details. The custom deleter in the singleton case for shared_ptr makes deletion a no-op, because the object owned by the shared_ptr is allocated statically. However, be aware of problems of destruction order of static variables: Static initialization order fiasco

Johannes Schaub - litb
Singletons are a great pattern to follow in cases like this, but you'll need to be aware of multithreading issues with Singletons if you're working with a multithreaded application.
twokats
No need to return a pointer. Return a reference. The user then does not need to validate it is not NULL.
Martin York
originally wanted to return a reference. but i felt bad about it since it's created by new. i don't associate refs with new in mind, it looks strange to me :) This way, they could just stuff it into a shared_ptr as is.
Johannes Schaub - litb
In the case of Singleton, the destructor should also be private so that the user cannot delete the pointer received. In the case of a Multiton, the destructor must be available to user code so resources can be freed.
David Rodríguez - dribeas
dribeas, oh right i forgot to make the dtor private -.- well i think he would come up with some macro anyway to make that. something like googles' DISABLE_EVIL_CONSTRUCTORS(ClassName); :p
Johannes Schaub - litb
"This way, they could just stuff it into a shared_ptr as is." Well, then, return it as a shared_ptr immediately tomake it crystal clear that the memory pointed at must be freed by someone.
Johann Gerell
Johann. no that would defeat the purpose of that design. we would have to return a shared_ptr for the singleton case too then.
Johannes Schaub - litb
well but i think i'll change this to return a shared_ptr, but for the statically allocated object (singleton), we use a no-op deleter. that's listed as an explicit usecase for a custom deleter
Johannes Schaub - litb
+2  A: 

It doesn't look like you need the global object to do the management, so why not move everything into the classes themselves?

template <class Type>
class BrokeredObject
{
protected:
    static Type *theInstance;

public:
    static Type *getOrCreate()
    {
        if (!theInstance) {
            theInstance = new Type();
        }

        return theInstance;
    }

    static void free()
    {
        delete theInstance;
    }

};

class TestObject : public BrokeredObject<TestObject>
{
public:
    TestObject()
    {}

};


int
main()
{
    TestObject *obj = TestObject::getOrCreate();
}
David Norman
The global object has a few other things it does. It actually handles feature configuration and whether an object type should be able exist or not depending on available hardware and other factors.
ctacke
So what if BrokeredObject::getInstance got the necessary information from some global configuration object?
David Norman
You expect people to manually call free()? Who calls it. Multiple objects may retrieve and store a copy of the pointer so who has the responsibility of calling free()!
Martin York
You could handle freeing lots of ways, e.g., register the objects with a global that would free them when the process exits, since he has a global object, anyway. Or, since they appear to be singletons just don't free them at all.
David Norman
A: 

You should almost certainly be using dependency injection.

Bill K
I'm fairly certain your right. Care to point me to something that helps get me there?
ctacke
Sorry, not a big C++ user and was speaking in generalities. When I looked into it, it looks like C++ has difficulty with a lot of the concepts that DI needs, but these here's a project that tried: http://code.google.com/p/autumnframework/
Bill K
A: 

Why not this?

    template 
    BrokeredObject* GetOrCreateObject()
    {
      return new T();
    }
+3  A: 

Use a template class as the broker.
Make the instance a static member of the function. It will be created on first use and automagically-destroyed when the program exits.

template <class Type>
class BrokeredObject
{
    public:
        static Type& getInstance()
        {
            static Type theInstance;

            return theInstance;
        }
}; 

class TestObject
{
    public:
       TestObject()
       {}
};


int main()
{
    TestObject& obj =BrokeredObject<TestObject>::getInstance();
}
Martin York
In a comment to another question, the author says: The global object has a few other things it does. It actually handles feature configuration and whether an object type should be able exist or not depending on available hardware and other factors.
David Norman
+1  A: 

If you always know the type at compile time there is little point in calling BrokeredObject* p = GetObjectByID(TypeA::get_InterfaceID()) instead of TypeA* p = new TypeA or TypeA o directly.

If you on the other hand don't know the exact type at compile time, you could use some kind of type registry.

template <class T>
BrokeredObject* CreateObject()
{
    return new T();
}

typedef int type_identity;
typedef std::map<type_identity, BrokeredObject* (*)()> registry;
registry r;

class TypeA : public BrokeredObject
{
public:
     static const type_identity identity;
};

class TypeB : public BrokeredObject
{
public:
     static const type_identity identity;
};

r[TypeA::identity] = &CreateObject<TypeA>;
r[TypeB::identity] = &CreateObject<TypeB>;

or if you have RTTI enabled you could use type_info as type_identity:

typedef const type_info* type_identity;
typedef std::map<type_identity, BrokeredObject* (*)()> registry;
registry r;

r[&typeid(TypeA)] = &CreateObject<TypeA>;
r[&typeid(TypeB)] = &CreateObject<TypeB>;

Each new class could of course, in any case, be self-registering in the registry, making the registration decentralized instead of centralized.

dalle
You would need to complete this with the broker's logic that was an added requirement in one other post's comment (the broker has logic to detect whether the object should or not be created). Anyway this is the approach I use for abstract factories, encapsulated in a simgleton object.
David Rodríguez - dribeas
+4  A: 

The way I would solve this problem is using what I would call the Static Registry Pattern, which in my mine mind is the C++ version of dependency injection.

Basically you have a static list of builder objects of a type that you use to build objects of another type.

A basic static registry implementation would look like:

template <class T>
class StaticRegistry
{
public:
    typedef std::list<T*>   Container;

    static  StaticRegistry<T>&  GetInstance()
    {
        if (Instance == 0)
        {
            Instance = new StaticRegistry<T>;
        }
        return *Instance;
    }

    void    Register(T* item)
    {
        Items.push_back(item);
    }

    void    Deregister(T* item)
    {
        Items.remove(item);
        if (Items.empty())
        {
            delete this;
            Instance = 0;
        }
    }

    typedef typename Container::const_iterator  const_iterator;

    const_iterator begin() const
    {
     return Items.begin();
    }

    const_iterator end() const
    {
     return Items.end();
    }

protected:
    StaticRegistry() {}
    ~StaticRegistry() {}

private:
    Container               Items;

    static StaticRegistry<T>*   Instance;
};

template <class T>
StaticRegistry<T>* StaticRegistry<T>::Instance = 0;

An implementation of BrokeredObjectBuilder could look like this:

class BrokeredObjectBuilderBase {
public:
    BrokeredObjectBuilderBase() { StaticRegistry<BrokeredObjectBuilderBase>::GetInstance().Register(this); }
    virtual ~BrokeredObjectBuilderBase() { StaticRegistry<BrokeredObjectBuilderBase>::GetInstance().Deregister(this); }

    virtual int GetInterfaceId() = 0;
    virtual BrokeredObject* MakeBrokeredObject() = 0;
};


template<class T>
class BrokeredObjectBuilder : public BrokeredObjectBuilderBase {
public:
    BrokeredObjectBuilder(unsigned long interface_id) : m_InterfaceId(interface_id) { } 
    virtual int GetInterfaceId() { return m_InterfaceId; }
    virtual T* MakeBrokeredObject() { return new T; }
private:
    unsigned long m_InterfaceId;
};


class TypeA : public BrokeredObject
{
   ...
};

// Create a global variable for the builder of TypeA so that it's 
// included in the BrokeredObjectBuilderRegistry
BrokeredObjectBuilder<TypeA> TypeABuilder(TypeAUserInterfaceId);

typedef StaticRegistry<BrokeredObjectBuilderBase> BrokeredObjectBuilderRegistry;

BrokeredObject *GetObjectByID(int id)
{
  BrokeredObject *pObject(0);
  ObjectMap::iterator = m_objectList.find(id);
  // etc.
  if(found) return pObject;

  // not found, so create
  BrokeredObjectBuilderRegistry& registry(BrokeredObjectBuilderRegistry::GetInstance());
  for(BrokeredObjectBuilderRegistry::const_iterator it = registry.begin(), e = registry.end(); it != e; ++it)
  {
    if(it->GetInterfaceId() == id)
    {
      pObject = it->MakeBrokeredObject();
      break;
    }
  }

  if(0 == pObject)
  {
    // userinterface id not found, handle this here
    ...
  }      

  // add it to the list
  return pObject;
}

Pros:

  • All the code that knows about creating the types is seperated out into the builders and the BrokeredObject classes don't need to know about it.
  • This implementation can be used in libraries and you can control on a per project level what builders are pulled into a project using a number of different techniques.
  • The builders can be as complex or as simple (like above) as you want them to be.

Cons:

  • There is a wee bit of infrastructure involved (but not too much).
  • The flexability of defining the global variables to include what builders to include in your project does make it a little messy to work with.
  • I find that people find it hard to understand this pattern, I'm not sure why.
  • It's sometimes not easy to know what is in the static registry at any one time.
  • The above implementation leaks one bit of memory. (I can live with that...)

The above implementation is very simple, you can extend it in lots of different ways depending on the requirements you have.

Shane Powell
wxWidgets uses this pattern or very similar for doing internal wxRTTI. You add a new widget, you don't have to change the wxRTTI to know about your widget because the static inheritance does it for you. you can the create a control by id.
Greg Domjan
A: 

My use-case tended to get a little more complex - I needed the ability to do a little bit of object initialization and I needed to be able to load objects from different DLLs based on configuration (e.g. simulated versus actual for hardware). It started looking like COM and ATL was where I was headed, but I didn't want to add the weight of COM to the OS (this is being done in CE).

What I ended up going with was template-based (thanks litb for putting me on track) and looks like this:

class INewTransModule
{
  public:
    virtual bool Init() { return true; }
    virtual bool Shutdown() { return true; }
};

template <typename T>
struct BrokeredObject
{
public:
    inline static T* GetInstance()
  {
    static T t;
    return &t;
  }
};

template <> 
struct BrokeredObject<INewTransModule>
{
public:
    inline static INewTransModule* GetInstance()
  {
    static INewTransModule t;
    // do stuff after creation
    ASSERT(t.Init());
    return &t;
  }
};

class OBJECTBROKER_API ObjectBroker
{
  public: 
    // these calls do configuration-based creations
    static ITraceTool  *GetTraceTool();
    static IEeprom     *GetEeprom();
    // etc
};

Then to ensure that the objects (since they're templated) actually get compiled I added definitions like these:

class EepromImpl: public BrokeredObject<EepromImpl>, public CEeprom
{
};

class SimEepromImpl: public BrokeredObject<SimEepromImpl>, public CSimEeprom
{
};
ctacke