Is it good practice to store copies of the same shared pointers in different vectors?

Question

I have a base class, BaseObject, and two derived class DerivedObject1 and DerivedObject2. They share a common behavior and methods, but DerivedObject1 has an additional method. My main class MyClass stores (in std::vector) boost::shared_ptr of instances of those classes. MyClass needs to call commonMethod() for all the BaseObject, and sometimes call additionalMethod() for all DerivedObject1.

class BaseObject
{
  virtual void commonMethod();
}

Class DerivedObject1 : public BaseObject
{
  void commonMethod();
  void additionalMethod();
}

Class DerivedObject2 : public BaseObject
{
  void commonMethod();
}

Are there any disadvantages of having two vectors in MyClass, one that stores ALL the pointers of DerivedObject1 and DerivedObject2, and another vector that stores only the pointers of DerivedObject1 ? Meaning I would have all the DerivedObject1 pointers twice. But I think the call to the different methods would be clear, at least.

class MyClass
{
  typedef std::vector<std::shared_ptr<BaseObject>> BaseObjectVector;
  typedef std::vector<std::shared_ptr<DerivedObject1>> DerivedObject1Vector;
  BaseObjectVector everything;
  DerivedObject1Vector only_derived1;

  void doSomething()
  {
    for (BaseObjectVector::iterator iter = everything.begin(); iter != everything.end(); ++iter)
    {
      (*iter)->commonMethod();
    }
  }

  void doSomethingForDerivedObject1()
  {
    for (DerivedObject1Vector::iterator iter = only_derived1.begin(); iter != only_derived1.end(); ++iter)
    {
      (*iter)->additionalMethod();
    }
  }
}

I can think of other ways to do this, mainly having one vector for DerivedObject1 and one vector for DerivedObject2, but to call commonMethod(), I would have to iterate over both vectors. My original solution seems the best to me, except that some pointers are stored twice. What are the disadvantages of this ?

Answer 1

Interesting question. We sometimes meet such a situation in maintenance of legacy codes which we don’t know who wrote.

Are there any disadvantages of having two vectors in MyClass … ?

I think there are no mechanical (or performance) disadvantages. If we are struggled to meet the release deadline, we have no choice but to pick such an simple way. However, storing same vectors twice actually lowers maintainability and we should consider improving it in the future.

---

• std::dynamic_pointer_cast (or boost::dynamic_pointer_cast) [Demo]

If you need dynamic_pointer_cast to implement functions like @Caleth ’s push_back / remove, how about removing only_derived1 and reluctantly applying just one dynamic_pointer_cast in doSomethingForDerivedObject1() from the get-go as follows ? It will make MyClass simpler. Required modification will not be complicated if DerivedObject3 is defined in the future.

void MyClass::doSomethingForDerivedObject1()
{
    for (const auto& obj_i : everything)
    {
      if (auto derived1 = std::dynamic_pointer_cast<DerivedObject1>(obj_i))
        {
           derived1->additionalMethod();
        }
    }
}

void MyClass::doSomethingForDerivedObject3()
{
    for (const auto& obj_i : everything)
    {
      if (auto derived3 = std::dynamic_pointer_cast<DerivedObject3>(obj_i))
        {
           derived3->additionalMethod();
        }
    }
}

---

• Dummy Method as proposed by @sagi [Demo]

Declaring virtual function BaseObject::additionalMethod() and implementing

void DerivedObject2::additionalMethod()
{ 
  /* nothing to do */
}

then you can again remove only_derived1. In this method you must implement DerivedObject3::additionalMethod() only if DerivedObject3 is defined.

But, although it depends on your constructor or setter code, if the following case will also occur

everything;    ->derived2
only_derived1; ->derived1

this method is still insufficient.

---

• Changing Design [Demo]

Ideally, we should not use public inheritance to implement objects within "IS-ALMOST-A“ relations, as Herb Sutter says. The relation between BaseObject, DerivedObject1 and DerivedObject2 looks like this one. Since I don’t know whole code of your application I may be wrong, but it will be worthwhile to consider extracting DerivedObject1::additionalMethod() as another class or a function pointer and putting its vector in MyClass as a private member.