0

I have two classes B and C (both derived from a class A) a class called H which holds either A or B.

Code :

class A  // abstract base class
{
  // bells and whistles
  // virtual fn 
     void fn() = 0 ; 
     protected:
         // some variables common to B and C
};

class B : public A  // inherit from A...
{
      B();
      fn(); // implement virtual function
      // bells and whistles
};

B::B() :A()
{
 // things
}

void B::fn()
{
 // things
}

// Class C : exactly the same as class B 


// Class H : holds B or C 
enum class CLASS_TYPE { TYPE_B, TYPE_C }

class H
{
   public:
      H(CLASS_TYPE type_);
      ~H();
   private: 
      A * _obj; 

};


H::H(CLASS_TYPE type_)
{
     switch(type_)
     {
     case CLASS_TYPE::B:
          _obj = new B();
          break;
     case CLASS_TYPE::C:
          _obj = new C();
          break;
     }
}

H::~H()
{
    delete _obj;// delete allocated object 
}

Is this the right way to create an internal object within a class based on arguments in a constructor ? Also is using abstract base class , virtual efficient ? This is part of a large scale simulation program , and I would like to avoid performance penalties.

Thank you. PS: I have tried to keep things simple, if there is any more information required please tell.

nnrales
  • 1,481
  • 1
  • 17
  • 26

2 Answers2

3

Is this the right way to create an internal object within a class based on arguments in a constructor ?

My first inclination is to get rid of the enum. Let the users of the class construct an object and use it to construct an H.

Add a virtual member function clone() to A.

Change the constructor to accept a A const&. Make a copy of the input object in the constructor using the clone() member function.

Change the member variable in H to a std::unique_ptr instead of a raw pointer.

Here's my suggestion:

class A 
{
   public:
      virtual A* clone() const = 0;
      virtual void fn() = 0 ; 
   protected:
};

class B : public A
{
   public:

      virtual A* clone() const
      {
         return new B;
      }

      B();
      virtual void fn();
};

B::B() : A()
{
}

void B::fn()
{
}

#include <memory> // of std::unique_ptr
class H
{
   public:

      H(A const& a);

      // No need to have an explicit destructor.
      // The compiler generated destructor will do just fine.
      // ~H();

   private: 
      std::unique_ptr<A> aPtr_;
};

H::H(A const& a) : aPtr_(a.clone())
{
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
0

You'll need

virtual void fn() = 0 ; 

in Class A and

void fn() override;  // note: override is not strictly need but good practice

in class B and C

Further you have to take care of copy and assignments as you have a raw pointer in the class. A direct member-by-member copy will lead to two H objects pointing to the same B/C. This will be very bad. So to be safe you should delete the default generated copy/assignment. A better approach is to avoid raw pointers if possible.

The performance cost of virtual functions (polymorphism) is low. Also see Cost of Polymorphic calls - C++

Community
  • 1
  • 1
Support Ukraine
  • 42,271
  • 4
  • 38
  • 63