We have a library with that publishes an abstract base class:
(illustrative psudocode)
/include/reader_api.hpp
class ReaderApi
{
public:
static std::unique_ptr <ReaderApi> CreatePcapReader ();
virtual ~ReaderApi() = 0;
};
In the library implementation, there is a concrete implementation of ReaderApi that reads pcap files:
/lib/pcap_reader_api.cpp
class PcapReaderApi
:
public ReaderApi
{
public:
ReaderApi() {};
};
Client code is expected to instantiate one of these PcapReaderApi objects via the factory method:
std::unique_ptr <ReaderApi> reader = ReaderApi::CreatePcapReader ();
This feels gross to me on a couple of levels.
First, the factory method should be free, not a static member of ReaderApi. It was made a static member of ReaderApi to be explicit about the namespaces. I can see pros & cons either way. Feel free to comment on this, but it's not my main point of contention.
Second, my instinct tells me I should be using std::make_unique rather than calling a factory method at all. But since the actual object being made is an implementation detail, not part of the public headers, there's nothing for the client to make_unique.
The simplest solution, in terms of understandability and code maintenance, appears to be the solution I've already come up with above, unless there is a better solution that I'm not already aware of. Performance is a not a major consideration here since, because of the nature of this object, it will only be instantiate once, at startup-time.
With code clarity, understandability, and maintainability in mind, is there a better way to design the creation of these objects than I have here?
I have considered two alternatives that I'll go over below.
One alternative I've considered is passing in some kind of identifier to a generic Create function. The identifier would specify the kind of object the client wishes to construct. It would likely be an enum class, along these lines:
enum class DeviceType
{
PcapReader
};
std::unique_ptr <ReaderApi> CreateReaderDevice (DeviceType);
But I'm not sure I see the point of doing this versus just making the create function free and explicit:
std::unique_ptr <ReaderApi> CreatePcapReader ();
I also thought about specifying the DeviceType parameter in ReaderApi's constructor:
class ReaderApi
{
public:
ReaderApi (DeviceType type);
virtual ~ReaderApi() = 0;
};
This would enable the make_unique idiom:
std::unique_ptr <ReaderApi> reader = std::make_unique <ReaderApi> (DeviceType::PcapReader);
But this obviously would present a big problem -- you're actually trying to construct a ReaderApi, not a PcapReader. The obvious solution to this problem is to implement a virtual constructor idiom or use factory construction. But virtual construction seems over-engineered to me for this use.
