Member functions for derived information in a class

Question

While designing an interface for a class I normally get caught in two minds whether should I provide member functions which can be calculated / derived by using combinations of other member functions. For example:

class DocContainer
{
 public:
   Doc* getDoc(int index) const;
   bool isDocSelected(Doc*) const;
   int getDocCount() const;

  //Should this method be here???
  //This method returns the selected documents in the contrainer (in selectedDocs_out)
  void getSelectedDocs(std::vector<Doc*>& selectedDocs_out) const;
};

Should I provide this as a class member function or probably a namespace where I can define this method? Which one is preferred?

Answer 1

I think its fine to have getSelectedDocs as a member function. It's a perfectly reasonable operation for a DocContainer, so makes sense as a member. Member functions should be there to make the class useful. They don't need to satisfy some sort of minimality requirement.

One disadvantage to moving it outside the class is that people will have to look in two places when the try to figure out how to use a DocContainer: they need to look in the class and also in the utility namespace.

Answer 2

The STL has basically aimed for small interfaces, so in your case, if and only if getSelectedDocs can be implemented more efficiently than a combination of isDocSelected and getDoc it would be implemented as a member function.

This technique may not be applicable anywhere but it's a good rule of thumbs to prevent clutter in interfaces.

Answer 3

The answer is proabably "it depends"...

If the class is part of a public interface to a library that will be used by many different callers then there's a good argument for providing a multitude of functionality to make it easy to use, including some duplication and/or crossover. However, if the class is only being used by a single upstream caller then it probably doesn't make sense to provide multiple ways to achieve the same thing. Remember that all the code in the interface has to be tested and documented, so there is always a cost to adding that one last bit of functionality.

Answer 4

In general, you should probably prefer free functions. Think about it from an OOP perspective.

If the function does not need access to any private members, then why should it be given access to them? That's not good for encapsulation. It means more code that may potentially fail when the internals of the class is modified.

It also limits the possible amount of code reuse.

If you wrote the function as something like this:

template <typename T>
bool getSelectedDocs(T& container, std::vector<Doc*>&);

Then the same implementation of getSelectedDocs will work for any class that exposes the required functions, not just your DocContainer.

Of course, if you don't like templates, an interface could be used, and then it'd still work for any class that implemented this interface.

On the other hand, if it is a member function, then it'll only work for this particular class (and possibly derived classes).

The C++ standard library follows the same approach. Consider std::find, for example, which is made a free function for this precise reason. It doesn't need to know the internals of the class it's searching in. It just needs some implementation that fulfills its requirements. Which means that the same find() implementation can work on any container, in the standard library or elsewhere.

Scott Meyers argues for the same thing.

If you don't like it cluttering up your main namespace, you can of course put it into a separate namespace with functionality for this particular class.

Answer 5

I agree with the answers from Konrad and jalf. Unless there is a significant benefit from having "getSelectedDocs" then it clutters the interface of DocContainer.

Adding this member triggers my smelly code sensor. DocContainer is obviously a container so why not use iterators to scan over individual documents?

class DocContainer
{
public:
  iterator begin ();
  iterator end ();

  // ...
  bool isDocSelected (Doc *) const;
};

Then, use a functor that creates the vector of documents as it needs to:

typedef std::vector <Doc*> DocVector;
class IsDocSelected {
public:
  IsDocSelected (DocContainer const & docs, DocVector & results)
  : docs (docs)
  , results (results)
  {}

  void operator()(Doc & doc) const
  {
    if (docs.isDocSelected (&doc))
    {
      results.push_back (&doc);
    }
  }
private:
  DocContainer const & docs;
  DocVector & results;
};


void foo (DocContainer & docs)
{
  DocVector results;
  std :: for_each (docs.begin ()
    , docs.end ()
    , IsDocSelected (docs, results));
}

This is a bit more verbose (at least until we have lambdas), but an advantage to this kind of approach is that the specific type of filtering is not coupled with the DocContainer class. In the future, if you need a new list of documents that are "NotSelected" there is no need to change the interface to DocContainer, you just write a new "IsDocNotSelected" class.

Answer 6

I think this is perfectly valid if the method:

• fits in the class responsibilities
• is not too specific to a small part of the class clients (like at least 20%)

This is especially true if the method contains complex logic/computation that would be more expensive to maintain in many places than only in the class.