3

Title edits that reflect a better summary of the question are welcome.

I'd like to refactor these three classes somehow to remove the duplicate field represented in class C (see hierarchy). I thought about pulling the field up into a parent class, but the issue is that A and B are not similar enough to be considered "is-a", C is considered both, and it is literally only one member field so creating a class just to hold ONE THING seems a bit overkill.

Hierarchy:

(abstract data type)
class A : public O {
    public:
    //...
    std::string GetName();
    std::string GetName() const;
    void SetName(std::string name);
    //...
    protected:
    //...
    std::string _name;
    //...
};

//Methods and fields shown here represent the exact same representative data as in A but the classes are so dissimilar as to not be considered "is-a" relationship.
(abstract data type)
class B {
    public:
    //...
    std::string GetName();
    std::string GetName() const;
    void SetName(std::string name);
    //...
    protected:
    //...
    std::string _name;
    //...
};

(concrete)
class C : public A, public B {
    public:
    //...
    C(/*..Other parameters..*/, std::string name, /*....*/)
    : A(name, /*...*/), B(name, /*...*/) {
        /*...*/
    }
    //...
    private:
    //...        
};
Casey
  • 10,297
  • 11
  • 59
  • 88
  • 1
    This seems perfectly acceptable, you're combining two classes that are not linked by parents to which you need to add new data/methods. Besides, they're both abstract so how else could you do it? – Andre Jun 16 '12 at 22:05
  • If you do not want to make a separate class for only one member then I think this design is already good. Over-optimizing might not be a good idea in this case. – A. K. Jun 17 '12 at 03:22

3 Answers3

1

You can either leave it as-is, as said previously or you can consider using composition over inheritance for class C like:

class C : public A
{
public:
    // ...
    // The GetName and SetName methods are inherited from A.

private:
    B* b;
};

or

class C
{
public:
    // ...

    std::string GetName();
    std::string GetName() const;
    void SetName(std::string name);

private:
    A* a;
    B* b;
};
npclaudiu
  • 2,401
  • 1
  • 18
  • 19
  • This is a better choice if there are no other classes that also want names. – jxh Jun 16 '12 at 23:21
  • I'm inclined to agreeing with user315052, even though this is the only class that inherits from both A and B, there are other classes that inherit just from A or just from B that need names. – Casey Jun 20 '12 at 21:05
0

Looking at this question and answers: Get rid of ugly if statements clearly shows that, as @Andre mentioned, your current code is perfectly acceptable and trying to "fix" it may cause same pain and mind blow.

Leave it AS-IS, it's good.

Community
  • 1
  • 1
dantuch
  • 9,123
  • 6
  • 45
  • 68
0

Since C is passing in the same name parameter to both A and B, you might be able to get what you want with virtual inheritance. V below is defined to be a common base class for A, B, and C, but with virtual inheritance, they all share the same instance.

class V {
    public:
    std::string GetName();
    std::string GetName() const;
    void SetName(std::string name);
    protected:
    std::string _name;
    V () {}
    V (std::string n) : _name(n) {}
    ~V () {}
};

class A : virtual public V, public O {
    //...
};

class B : virtual public V {
    //...
};

class C : virtual public V, public A, public B {
    public:
    C (/*...otherargs,*/std::string name/*,moreargs...*/)
        : V(name), A(/*...*/), B(/*...*/) {
        //...
    }
    //...
};
jxh
  • 69,070
  • 8
  • 110
  • 193