views:

137

answers:

4

The gist of this question is about extending a class, minimizing jam-packing everything into a single class, and maximizing code re-use. After reading this question, please feel free to edit the title or description to make it more succinct. Though the post looks long, I am just trying to be thorough by using a lot of examples.

Suppose I have a class:

class UsedByManyPeople
{
  // ...has many fields
};

As the name implies, this class is used by a lot of developers. I have to add 2 features to this class:

  1. a convert() that converts UsedByManyPeople to SomeOtherType
  2. a getFileName() that returns a string

Both of them are specific to my department's needs.


First attempt solution

At first I thought about simply adding 2 new methods to UsedByManyPeople.Thus, the class will now look like:

class UsedByManyPeople
{
  // ...has many fields

  public:
    SomeOtherType const convert() const;
    std::string   const getFileName() const;
};

However, the 2 features are actually specific to my department's use case, and other departments do not even have the class definition of SomeOtherType nor do they care about the getFileName().

Clearly, the above approach is not a good approach (?).

How would you extend this class?

Alternatives that came to my mind:


Subclass UsedByManyPeople and create my own class.

  • Tie data and method together

For example,

class ExtUsedByManyPeople : public UsedByManyPeople
{
  public:
    SomeOtherType const convert() const;
    std::string   const getFileName() const;
};

Create Helper classes, one for each method (yikes!), and implement it as static methods.

  • Separate data from methods, single class one responsibility

For example,

class UsedByManyPeopleToSomeOtherTypeConverter
{    
  public:
    static SomeOtherType const convert(UsedByManyPeople const&);
};
class UsedByManyPeopleFileName
{
  public:
    static std::string const getFileName(UsedByManyPeople const&);
};

Create a single Helper class, with all the methods inside.

  • Separate data from methods, single class many responsibilities

For example,

class UsedByManyPeopleHelper
{
  public:
    static SomeOtherType const convert(UsedByManyPeople const&);
    static std::string   const getFileName(UsedByManyPeople const&);
};
+2  A: 

the last option won't affect anyone else, and isolates your one-off needs into a single class

Steven A. Lowe
What's your opinion on the first option ? inheritance.
ShaChris23
If there are some private or protected members of UsedByManyPeople that you'll need to do your conversion, it may be easier to use inheritance.
thebretness
@[ShaChris23]: inheritance would work, but then your department would have to use the inherited class everywhere, and if there's any shared code that uses the base class you'd be unable to do the conversion on it. The last option is the least-impact.
Steven A. Lowe
+3  A: 

Especially if the methods are specific to your departments use of the class you should implement them as in: Create a single Helper class, with all the methods inside.

There are several reasons for this:

  • The single helper class can be located in another logical project structure
  • If your new methods don't require access to the internal state of the class this is expressed clearly. It provides better encapsulation. (which you supplemented by const ref, which is think is a good style)
  • UsedByManyPeople shouldn't be responsible for converting itself into another type. This violates SOLID
Johannes Rudolph
What's your opinion on the first option ? inheritance.
ShaChris23
The single helper class can only access public members of UsedByManyPeople. If the helper class can't do its work with just publicly available data and functions, inheritance is probably a beter option.
thebretness
In general you should prefer composition over inheritance. If you need access to the private state of the object use inheritance or consider making them friends.
Johannes Rudolph
+1  A: 

You shouldn't change the existing UsedByManyPeople class. If your department has preferences on what patterns to use, then stay within your department's style. Otherwise, create the helper class for conversions. If you see multiple conversions down the road you may want to create a class for each conversion. For the getFileName extension, you might want to derive a class (if this is in fact a useful extension) or add it to your helper.

Again, your department's standard (if one exists) should guide you on this. If you don't have a standard, now's the time to create one.

Liz Albin
+1  A: 

Although this is almost identical (in usage at least) to the class with public static methods, you could use a namespace to encapsulate the routines you need (as said before, this will only work provided the calls can be implemented from the public data and methods of the original class).

namespace MyDept
{
    SomeOtherType const convert( UsedByManyPeople const& );
    std::string   const getFileName( UsedByManyPeople const& );
}

which provides a very similar interface to the caller as the static calls from a class, for instance

overusedObjectAsSomeOtherType = MyDept::convert( overusedObject );

I'd like to hear from the other people that have answered on this list as to whether there are any real differences between this approach and the one that was overwhelmingly selected.

bpw1621