C++: aggregation, inheritance and pointers

Question

This is a question about particular OO implementation in C++ of the given problem:

There are several variants of an algorithm, say two, both of which have some generic parts. These algorithms (their implementations, AlgImpl) receive startup options. They also receive input data, that they process, piece by piece. There are different ways the startup options can be supplied (from file, from network, from user input), as well as there are different ways how the data are received (from file, from device). There is a common API for different sources of data, similarly for the options.

The architecture should allow for use of any available AlgImpl together with any available sources of options and data. It should also allow new AlgImpl's and possibly new types of sources for options and data to be added with minimum or none changes to the original code (with say only two AlgImpl's, two option sources types and two data sources types).

Here is what I think of it in terms of C++, using inheritance, aggregation and pointers. Since all AlgImpl's share some common parts, it is naturally to organize them around a base abstract class, so (omitting non-essential data types) we have:

class BaseAlgorithm
{
   ...  //Abstract class with common code
};

class SimpleAlgorithmImpl: public BaseAlgorithm
{
   ...
};

class OptimalAlgorithmImpl: public BaseAlgorithm
{
   ...
};

Now, the options and data sources have the same interface respectively:

class BaseOptionsSource
{
public:
   //Interface
   virtual void GetOptions(Options& opts) = 0;
};

class FileOptionsSource: public BaseOptionsSource
{
   void GetOptions(Options& opts);
   ...
};

class NetworkOptionsSource: public BaseOptionsSource
{
   void GetOptions(Options& opts);
   ...
};

class BaseDataSource
{
public:
   //Interface
   virtual void GetDataChunk(DataChunk& chunk) = 0;
};

class FileDataSource: public BaseDataSource
{
   void GetDataChunk(DataChunk& chunk);
   ...
};

class DeviceDataSource: public BaseDataSource
{
   void GetDataChunk(DataChunk& chunk);
   ...
};

Options and data sources are then made members of BaseAlgorithm:

class BaseAlgorithm
{
public:
   BaseAlgorithm(BaseDataSource* pDataSrc, BaseOptionsSource* pOptsSrc); 
   BaseDataSource* _pDataSrc;
   BaseOptionsSource* _pOptsSrc;
};

BaseAlgorithm::BaseAlgorithm(BaseDataSource* pDataSrc, BaseOptionsSource* pOptsSrc):
      _pDataSrc(pDataSrc), _pOptsSrc(pOptsSrc)
{   
}

A partucular algorithm object can then be created as follows:

DeviceDataSource dataSrc;
NetworkOptionsSource optsSrc;
SimpleAlgorithmImpl simpleAlg(&dataSrc, &optsSrc);

For a new AlgImpl or new type of source, its class should implement inherited methods. Of course, there will have to be an "if/else if/..." code, that explicitly selects the set of used AlgImpl and sources before AlgImpl object can be created.

Source objects can also be reused in this case, provided that AlgImpl object does not manage allocation/deallocation of source objects passed to it.

Do you think this is the right way to do it in C++? Or maybe there exists some other, more simple, more flexible or less "problem-free" pattern for this kind of interchangeble sub-functionality implementation? Is it unavoidable to use pointers in this case?

Answer 1

Looks reasonable to me. I think pointers are fine if you can be confident the data source and options source are alive during the lifetime of the algorithm. I don't think pointers are unavoidable though. Here are few other options (some of which are C++11):

References

Preferably const references. Possibly a bit safer than pointers if you are passing them in the constructor and you don't need them to be nullable.

BaseAlgorithm(BaseDataSource& dataSrc,
              BaseOptionsSource& optsSrc) :
    dataSrc_(dataSrc),
    optsSrc_(optsSrc) {}

Shared smart pointers

If you can't be confident that the data source or options source are alive for the lifetime of the algorithm consider passing in a std::shared_ptr or std::weak_ptr. Use std::shared_ptr if you want to keep them alive, std::weak_ptr if you want to know if they are alive.

BaseAlgorithm(std::weak_ptr<BaseDataSource> dataSrc,
              std::weak_ptr<BaseOptionsSource> optsSrc) :
    dataSrc_(dataSrc), 
    optsSrc(optsSrc) {}

Sink

It depends on how these data sources, options sources and algorithms are created and used but maybe it will make ownership simpler if the algorithm takes ownership of the data source and/or options source and pass by unique_ptr.

BaseAlgorithm(std::unique_ptr<BaseDataSource> dataSrc,
              std::unique_ptr<BaseOptionsSource> optsSrc) :
    dataSrc_(std::move(dataSrc)),
    optsSrc(std::move(optsSrc)){} 

A few minor points: I question if an out parameter is the best way of getting options and data chunks. Don't be afraid of return-by-value even for something large like a data chunk The copy will always be optimized away by any modern compiler and it makes the calling code easier to read and write:

Options opts = optsSrc_->GetOptions(opts);
DataChunk data = dataSrc_->GetDataChunk(data);

You may have missed them out to simplify your post but I wonder if GetOptions and/or GetDataChunk can be const? Should calling them make changes to the source? If they can be const it means the algorithms only need a const pointer/reference to the source and it might make finding errors slightly easier. You talk about re-using source objects, this may not be wise if the algorithms can make changes to the source objects. For example what happens if you call GetDataChunk repeatedly, do you get a different chunk each time? This might have unexpected consequences if you can use the same data source in different algorithms.

Lastly, make sure _pOptsSrc and _pDataSrc are protected or private not public.