Replacing a class without deleting the old one, avoiding access duplication

Question

In a refactoring task, I have an old class, called OldClass, that will be replaced with the newer better NewClass.

OldClass and NewClass share have a lot in common, and for example share data members with the same name like memberA, memberB... But while they have the same name, these are members of OldClass and NewClass and are not shared through inheritance (nor do I want them to be shared).

They do have a common ancestor, so it would look like this:

      ┌──────────────┐
      │CommonAncestor│
      └───────▲──────┘
      ┌───────┴──────┐
      │         ┌────┴────┐
      │         │NewParent│
      │         └────▲────┘
┌─────┴─────┐  ┌─────┴──────┐
│ OldClass  │  │ NewClass   │
├───────────┤  ├────────────┤
│int memberA│  │int memberA │
│int memberB│  │bool memberB│
└───────────┘  └────────────┘

My problem is that both will exist within the code for a while before the transition is fully over, with a setting at start enabling the switch between old and new mode.

That means that in a lot of place, I will have

 CommonAncestor* c = ...;
 if(newMode) {
     NewClass* n = dynamic_cast<NewClass*>(c);
     n->setMemberA(...);
     n->setMemberB(...);
 }
 else {
     OldClass* o = dynamic_cast<NewClass*>(c);
     o->setMemberA(...);
     o->setMemberB(...);
 }

Which is awful (code duplication, and cast). (Of course, there is a lot more than two members...)

How can I avoid that? What would be a nice way for them to coexist without duplicating every member access and assignation?

The idea is that deleting OldClass, later, should be as easy as possible.

---

Side note: In the case of instanciation, I avoided duplication by being able to construct an OldClass member from a NewClass one, so:

 CommonAncestor* c;
 NewClass* n;
 n->setMemberA(...);
 n->setMemberB(...);
 if(newMode) {
    c = n;
 }
 else {
     OldClass* o(n); //we can make an Old by copying a New
     c = o;
 }

Answer 1

When you find yourself down-casting with dynamic_cast, something has usually gone wrong in your design:

CommonAncestor* c = ...;
if(newMode) {
    NewClass* n = dynamic_cast<NewClass*>(c);
    n->setMemberA(...);
    n->setMemberB(...);
}

This above example could instead be written as:

CommonAncestor* c = ...;
c->setMemberA(...);
c->setMemberB(...);

This would require setMemberA and setMemberB to be virtual member functions, so calling the base class functions chooses an implementation from the derived class. Even if you weren't allowed to modify CommonAncestor, you could:

• create a new class OldOrNewClass containing virtual void setMemberA(...), et al.
• make OldClass inherit from OldOrNewClass
  • OldOrNewClass can inherit from CommonAncestor, so that OldClass has only one parent
• make NewClass or NewParent inherit from OldOrNewClass
• downcast your CommonInterface* to OldOrNewClass* and then call the virtual member functions

CommonAncestor* c = ...;
OldOrNewClass* n = dynamic_cast<OldOrNewClass>(c);
n->setMemberA(...);
n->setMemberB(...);

Multiple inheritance is allowed in C++, so you can keep your old class hierarchy and add OldOrNewClass to that. However, if you are allowed to modify CommonInterface, you can save yourself this trouble.

Also see Why do we need virtual functions in C++?

Answer 2

Templates are for code deduplication:

template <typename T, typename F>
void manipulateClass(CommonAncestor* c, F&& functor) { // please give me a better name
  if (T* n = dynamic_cast<T*>(c)) {
    std::forward<Functor>(functor)(n);
  } else {
    throw std::invalid_argument("Class has incorrect type for selected mode");
  }
}

...

auto const setter = [&](auto n) { // all variables captured
  n->setMember(...);
  n->setMember(...);
  // ...
};
if (newMode)
  manipulateClass<NewClass>(c,setter);
else
  manipulateClass<OldClass>(c,setter);

A similar thing can be done for construction. Also, I would recommend using either std::unique_ptr<Class> or std::shared_ptr<Class> instead of Class*.