Ganesh M
Ganesh M

Reputation: 3706

C++ design issue

Here is the problem description of my design. There is a class A (Singleton) which creates and maintains objects of class B. But there is such scenario where if a particular condition hit in object of class B, it has to create another object of class B. But I need this creation of object to be done by class A.

 

class B;
class A {

     private:
          A();
          class A *ptr;
     public:
         A & GetAInstance()
         {
           if (!ptr)
               ptr = new A;
           return ptr;
         }
         void CreateBInstances(std::string name)
         {
              map[name] = new B(name);
         }
};

Class B {
       public:
           B(std::string name) { }
       public:
           void checkCondition()
           {
                if  (condition == true)
                {
                   // So here the contidition is hit, I want to create
                   // another object of class B, but I want to intimate
                   // class A to do this job for me
                }
           }
};

I would like to understand and looking for a better approach to do this dirty job. Thanks in advance.

Upvotes: 2

Views: 323

Answers (3)

Pete Kirkham
Pete Kirkham

Reputation: 49311

Much of the fluff about circular dependencies out there is confused when applied at implementation mechanisms rather than at package or module level, and some of it comes out of deficiencies in testing frameworks for Java. Because of the Java influence on the meme, solutions are phrased in the idioms of that rather than C++. It's not clear whether you're trying to remove a concrete circular dependency ( the code won't compile in C++ ) or have metaphysical objections.

One idiom for decoupling concrete circular dependencies is found in allocators in the C++ standard library.

When you declare a list so:

std::list < int, my_allocator < int > >

the allocator has a nested struct which allows access to the raw template for different specialisations so the std::list implementation can allocate node objects rather than just ints.

Assuming that you have the following requirements:

  • the registry is global to the program ( ie. you don't need more than one registry per object type, otherwise you need something like the factory pattern SingleShot proposes, though in C++ you'd more normally use template rather than virtual function polymorphism ); as such I tend to use static factory rather than singletons.
  • objects of class B should only be created by calling A::CreateInstance(name)
  • A acts as a registry, so repeated calls to create instance with the same name return the same object
  • code compiles correctly without concrete circular references
  • it is possible to replace the type of either the registry or registered type for testing

This global registry doesn't require any knowledge of the types it creates, other than that they supply a constructor taking a reference to a const std::string:

#include <string>
#include <map>

template < class T = int >
class Registry {
    static std::map<std::string,T*> map;

    public:

    static T& CreateInstance ( const std::string& name ) {
        typename std::map< std::string, T* >::iterator it = map.find ( name );

        if ( it == map.end() )
            return * ( map [ name ] = new T ( name ) );
        else
            return *it->second;
    }

    public:

    template < typename U > 
    struct rebind {
        typedef Registry<U> other;
    };
};

template  < class T >
std::map < std::string, T* > Registry<T>::map;

The corresponding registered object supplies a private constructor and has the CreateInstance function as a friend:

template < typename R = class Registry<> >
class Registered {
        typedef typename R::template rebind< Registered < R > > ::other RR;

    private:
        friend Registered<R>& RR::CreateInstance ( const std::string& name );

        explicit Registered ( const std::string& name ) {
            // ...
        }

        Registered ( const Registered<R>& ) ; // no implementation

    public:
        void checkCondition()
        {
            bool condition = 7 > 5;

            if  ( condition )
            {
                RR::CreateInstance ( "whatever" );
            }
        }

    // ...
};

Because of the rebind::other idiom, you don't have to write Registered<Registry<Registered<Registry ... and avoid the concrete circular dependency. Because the default Registry<int> is never actually used except to supply rebind, it's not instantiated and so doesn't report an error that you can't construct an int using new int ( name ).

You can then use the types for your B and A:

typedef Registered<>  B;
typedef Registry<B>   A;

int main () {
    B& b1 = A::CreateInstance("one"); // create a B

    b1.checkCondition(); // uses A to create object internally

    B b2("two"); // compile error - can only create B using A

    return 0;
}

You can of course construct a Registered< MyMockRegistry > for testing, the other main objection to circularly dependent types.

Upvotes: 2

SingleShot
SingleShot

Reputation: 19131

Something doesn't smell right about the whole design, but its hard to say what without having more details provided. Anyway, you could do something like this which avoids a cyclic dependency but still has B call A:

class BRegistry
{
   virtual void CreateBInstances(std::string& name) = 0;
   ...
};

class A : public BRegistry
{
   public:

      virtual void CreateBInstances(std::string& name) {
         map[name] = new B(name, *this);
      }

      // Singleton stuff...

      ...
};

class B
{
   public:

      B (std::string& name, BRegistry& registry) {
         ...
      }

      void checkCondition()
      {
         if  (condition == true)
         {
            registry.CreateBInstances(name);
         }
      }

      ...
};

Basically, I extract an interface from A and have B use the interface. A passes itself to B at creation time, but from B's point of view it is the interface. No cycles.

Just a comment. 99% of the time when I see Singleton, it is an inappropriate use of the pattern. Usually it is misused as just a convenient global variable, as exhibited by all the comments that jumped to say to use it from within B.

Upvotes: 1

Martin
Martin

Reputation: 3753

A::GetAInstance should be static.

Then you should be able to just do the following

if (condition)
{ 
    A::GetAInstance().CreateBInstance(name);
}

Upvotes: 5

Related Questions