RAII and code duplication in constructor/destructor

Question

I have a class hierarchy similar to this:

struct VideoCapture { virtual open(const string& uri) = 0; virtual close() = 0; };
struct VideoCapture1 : public VideoCapture {...};
struct VideoCapture2 : public VideoCapture {...};
struct VideoCapture3 : public VideoCapture {...};

I added the following constructor and destructor in each of these classes:

VideoCaptureN(const string& uri) { open(uri); }
~VideoCaptureN() { if (!isOpen()) { close(); } }

Though, providing RAII this did not pass the code review as I had to duplicate the same exact code in every classes.

What would be the solution to get RAII and avoid code duplication?

Answer 1

You can't call virtual functions in constructors and should not call them in destructors (you may have already destroyed state that the virtual function relies on).

You can use the PIMPl pattern to help you out though (like the c++ stream, they use a wrapper class and a buffer class that does the work).

class VidCaptureInterface
{
 public:
 virtual ~VidCaptureInterface() {} 
 virtual open(string const& uri) = 0;
 virtual close() = 0;
};
// When C++17 comes along then
// You may be able to force T to be a VidCaptureInterface
// This is one solution. But you basically want interface
// to be derived from `VidCaptureInterface`. In C++ stream
// the buffer is held as a pointer and can be re-assigned
// So alternatives include using some form of re-assignable pointer.
template<typename T>
class VideoCapture
{
 T interface;
 public:
 VideoCapture(std::string const& uri)
 {
 interface.open(uri);
 }
 ~VideoCapture()
 {
 if (isOpen())
 {
 interface.close();
 }
 }
};
typedef VideoCapture<VidCaptureInterface1> VideoCapture1;
typedef VideoCapture<VidCaptureInterface2> VideoCapture2;
typedef VideoCapture<VidCaptureInterface3> VideoCapture3;

• Ugo: Ok looks good to me. That is exactly the solution I started writting. Thanks
• Yuushi: You could always add a static_assert(std::is_base_of<VidCaptureInterface, T>::value);

Answer 2

You can't really help the constructor but you just need to make the destructor of VideoCapture virtual and let it do the closing:

struct VideoCapture { virtual open(const string& uri) = 0; virtual close() = 0; 
virtual ~VideoCapture (){if (!isOpen()) { close(); } }
};

Actually if there is any virtual function you should include a virtual destructor so proper cleanup can happen if you delete a VideoCapture and it happens to be a VideoCapture1 (otherwise the destructor of VideoCapture1 is never called).

• Ugo: I don't think this would work as calling virtual methods in constructor/destructor is not likely to work as expected. It won't call the code in the derived class.
• ratchet freak: @Ugo it will call the derived class' code, you just need to ensure that by the time it does so all data remains valid
• Ugo: Ah true you were speaking about the destructor only. Also this is still a bit risky.