Can't think of a good title, but you should read me anyway

[AYT]

Hi all,

I have a number of classes which are very different from each other but have *some* shared functionality. For example, some of them have an identifier, some of them have a name, some of them have a description, etc. I've created Interfaces like "Identifiable," "Nameable," and "Describable" to pool the shared functionality. For instance:

class Nameable {
public:
       Nameable(const std::string& name) : _name(name) {
       }
       const std::string& GetName() const {
               return _name;
       }
protected:
       std::string _name;
};

Then other classes can implement them. For instance, "items" have an id and a name, so I have:

class Item : public Identifiable, public Nameable {
       //...
};

Now I have a problem when I want to set two instances of a class which implements some of those interfaces equal to each other:

Item a(...);
Item b;
//...
b = a;

The problem is I have to have the following code inside Item:

Item& operator=(const Item& cpy) {
       _name = cpy._name; //Nameable
       _id = cpy._id; //Identifiable

       //Other copying stuff...
}

I don't want to custom-tailor operator= for every different class depending on which interfaces it implements. I'd like to have the functionality of copying Nameable things inside Nameable, copying Identifiable things inside Identifiable, etc.

My attempted solution was to add an "operator=" to the interfaces:

Nameable& operator=(const Nameable& cpy) {
       _name = cpy._name;
}

Then I figured I could try to call those operators inside the Item operator= function as follows:

Item& operator=(const Item& cpy) {
       *reinterpret_cast<Nameable*>(this) = cpy;
       *reinterpret_cast<Identifiable*>(this) = cpy;

       //Other copying stuff...
}

Then the Item operator= still has to be custom-tailored depending on which interfaces it implements, but I figured it was still better because it was encapsulating the copying for each interface, so if I had a very complex interface with a lot of data then it would still be very simple to copy, and it would be good code reuse, and if I decided to change the interface later on then I wouldn't have to modify every single class which implemented it. Unfortunately my attempted solution doesn't work. Here's a compilable example which demonstrates why it doesn't work:

#include <iostream>
using namespace std;

class A1 {
public:
       A1(int a) : a1(a) {}
       A1& operator=(const A1& cpy) {
               a1 = cpy.a1;
               return *this;
       }
       int a1;
};

class A2 {
public:
       A2(int a) : a2(a) {}
       A2& operator=(const A2& cpy) {
               a2 = cpy.a2;
               return *this;
       }
       int a2;
};

class B : public A1, public A2 {
public:
       B(int a1, int a2) : A1(a1), A2(a2) {}
       B& operator=(const B& cpy) {
               *reinterpret_cast<A1*>(this) = cpy;
               *reinterpret_cast<A2*>(this) = cpy;
               return *this;
       }
};

int main(int argc, char* argv[]) {
       B b1(5, 10);
       B b2(0, 0);
       b2 = b1;
       cout << b2.a1 << ", " << b2.a2 << endl;
}

I'd like it to output 5, 10, but instead it outputs 10, 0, because line 29 calls operator= from the A1 class. For this simple example where the type signatures match up, I end up with a neat (albeit wrong) answer. If I were to do this with my actual interfaces then I don't know what I'd get (garbage? memory access violation?).

Is there some sort of good solution to this problem (by which I mean, any way to have the functionality for copying inside the interfaces. I don't only mean a way to fix my specific failed solution, although that would work too) or will I just end up having to add copying code to all the classes that implement any of the interfaces?

[Disch]

Don't use reinterpret_cast like this. Bad bad bad for many reasons. Given that you have multiple inheritance, this is extremely likely to screw up. static_cast would be what you want for this, but even that is sketchy.

The bad news is that since the = operator isn't inherited, you pretty much have to overload it for every class. The good news is you can explicitly call parent class members by using the scope operator (::). You can also use the operator keyword to actually invoke the function, rather than actually using the = operator syntax.

Try this instead:

class B : public A1, public A2 {
public:
       B(int a1, int a2) : A1(a1), A2(a2) {}
       B& operator=(const B& cpy) {
               A1::operator = (cpy);
               A2::operator = (cpy);
               return *this;
       }
};

Or if you don't like that syntax, you can make a protected 'Assign' function or something similar and call it instead.


All of that said... "having a name" isn't much functionality to build a class around unless you're doing some complex stuff with names. I'd probably just give each class its own name member. Maybe even make it public and forgo the get/set methods (there's nothing wrong with public data members). But it's hard to say for sure what the best approach is without knowing the full context.


EDIT:

On a side note: reinterpret_cast was the cause of your problem. static_cast works as you'd expect. Though I still don't recommend the casting route.

Refrain from reinterpret_cast unless you're SURE it's what you want to do (which, it usually isn't).

[AYT]

That works perfectly, thank you.

As for building a class around having a name ... I included the Nameable and Identifiable examples because they were the simplest; I have more complicated interfaces as well. It's also simply more convenient for me because there are currently 5 Nameable classes and I might add more in the future, and it's nice to be able to just add a "public Nameable" instead of having to retype or copy that code every time, even if it is only 5 or 10 lines.