Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have materials:

class Material {...};
class ConcreteMaterialA : public Material {...};
class ConcreteMaterialB : public Material {...};

I want it to have a name:

class WithName
{
public:
    const string& Name() const          {return m_name;}
    void Rename(const string& name)     {m_name = name;}

private:
    string m_name;
}

And tracking its users:

template<class UserTy>
class WithUsers
{
public:
    void AddUser(T* user)       {m_users.push_back(user);}
    const std::vector<T*>& Users() const    {return m_users;}

private:
    std::vector<T*> m_users;
};

And wrapped with a pointer:

template<class T>
class WithPtr
{
public:
    template<class U> T* Get() const;
    template<class U> void Reset(U* u);
    template<> T* Get<T>() const            {return m_object.get();}
    template<> void Reset<T>(T* t)          {return m_object.reset(t);}

private:
   std::unique_ptr<T> m_object;
};

Then I define my new material:

class NewMaterial 
  : public WithPtr<Material>, public WithName, public WithUsers<MaterialUserType>
{}

It is kind of complex. I have many types that need a name, tracking users. Is it a good design?

share|improve this question

migrated from stackoverflow.com Dec 29 '12 at 15:26

This question came from our site for professional and enthusiast programmers.

1  
Why does "name" have to have it's own class? It's just a string, so there's no need to have it in a class as a resource cleanup item. Just stick string name as a member, and a getName() function in the classes that have names. Unless you specifically need some sort of "do this to all classes that have names" method somewhere. I think classes should be self-contained units as much as possible, but making things TOO small is just as big a problem as making them large. –  Mats Petersson Dec 28 '12 at 23:13
    
This looks crazy complicated, perhaps you could give more details about why you think this is a good idea. –  Winston Ewert Dec 29 '12 at 23:50
add comment

1 Answer 1

It looks like what you have implemented are mixins. In the world of c++ with multiple inheritance, I think it is a good design decision as long as you make sure that the method names you are mixing in don't start overlapping - read: that your number of mixins doesn't get out of hand.

Another alternative would be to use composition instead of inheritance. Instead of extending all the classes from which you want to reuse functionality, you could keep fields which point to such instances instead.

In general, you should make sure that the functionality you are trying to reuse is complex enough to justify the overhead of inheritance/composition. For instance, your "name" mixin seems to me to be too simple and thus not worth the effort.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.