C++ – Possible Alternatives to Copy Constructors

clibrariespointersraiismart-pointer

In my C++ project I am relying on some libraries that do memory management for me. I make wrapper classes, for ease of use and memory safety, for example the class below. Note that this is a much simplified example, to demonstrate my problem.

#include <library>

class Wrapper {
private:
    lib_type* data;
public:
    Wrapper() : data(library_new()) {
    }
    Wrapper(const Wrapper& orig) = delete;
    ~Wrapper() {
        library_free(data);
    }
    const lib_type* getData() const {
        return data;
    }
    /* ... */
    /* Lots of functions for using the wrapped object */
};

I needed to delete the copy constructor, because otherwise a copy going out of scope would invalidate the original object. Not being allowed to have copies makes the object very impractical to use – everywhere it is used I now need to hold a reference, and I need to manage the object centrally, which partially defeats the purpose of the wrapper class.

Possible solutions I have tried/thought of:

  • Copying the data. This is normally not an option though, because it is either not allowed by the library or the data is huge.
  • Move constructors. I tried solving the lack of a copy constructor by using a move constructor, but depending on the implementation, either this did not solve the problem or it became effectively a copy constructor, reintroducing the related problems.
  • Smart pointers. Then I realized it might be solved by using smart pointers to the wrapper instead of references, as this removes the need to maintain a central copy.
  • Wrapping a smart pointer. Or I could wrap a smart pointer to the data instead of a raw pointer. This makes the implementation of the wrapper a little bit more complicated, but also makes it easier to use, giving the classes using it a cleaner interface.

My attempt at the second smart pointer solution applied to the example above:

#include <library>

class Deleter {
public:
    void operator()(lib_type* p) const {
        library_free(p);
    }
};

class Wrapper {
private:
    shared_ptr<lib_type> data;
public:
    Wrapper() : data(library_new(), Deleter()) {
    }
    Wrapper(const Wrapper& orig) : data(orig.data) {
    }
    ~Wrapper() {
    }
    const lib_type* getData() const {
        return *data;
    }
    /* ... */
    /* Lots of functions for using the wrapped object */
};

Now for the concrete question: is this a good solution? Or are there other, perhaps better solutions?

I am particularly worried about the getData implementation; whether I should return a copy of the shared pointer or a naked pointer and why.

Best Answer

I think these implementations are reasonable and a generally good solution. Adding an appropriate move constructor and move assignment may help deal with your copy concerns - the default should be appropriate with the shared wrapper.

Some may argue (or advise) that you do not need to wrap the Standard Library facilities that you use here; whilst this is true, the semantics of the getData() function may be quite specific for your target code - using the library facility or wrapping it is really a design tradeoff.

You may need a few tweaks on the assignment operators; and possibly be more explicit on the other special member functions (I would favour this here - it makes the intent of the code clearer). By way of example;

#include <library>

class UniqueWrapper {
private:
    lib_type* data;
public:
    UniqueWrapper() : data(library_new()) {}
    UniqueWrapper(const UniqueWrapper&) = delete;
    UniqueWrapper& operator=(const UniqueWrapper&) = delete;
    UniqueWrapper(UniqueWrapper&&) = delete;
    UniqueWrapper& operator=(UniqueWrapper&&) = delete;

    ~UniqueWrapper() {
        library_free(data);
    }
    const lib_type* getData() const {
        return data;
    }
};

And as a shared wrapper;

#include <library>

class Deleter {
public:
    void operator()(lib_type* p) const {
        library_free(p);
    }
};

class SharedWrapper {
private:
    shared_ptr<lib_type> data;
public:
    SharedWrapper() : data(library_new(), Deleter()) {}

    SharedWrapper(const SharedWrapper& orig) = default;
    SharedWrapper& operator=(const SharedWrapper& orig) = default;
    SharedWrapper(SharedWrapper&& orig) = default;
    SharedWrapper& operator=(SharedWrapper&& orig) = default;

    ~SharedWrapper() = default;

    const lib_type* getData() const {
        return data.get(); // a reference return could also be used.
    }
    const lib_type& getDataAlt() const {
        // alternative for a reference
        return *data;
    }
};

On your last question;

I am particularly worried about the getData implementation; whether I should return a copy of the shared pointer or a naked pointer and why?

When you expose some of the internals of an object, you give up some control over how that data is used - essentially this is not a bad thing, you just need to bear that in mind.

Having said that, there are ways you can make the code easy to use properly and hard to use incorrectly. Returning a const in this case would probably be advised, since the method is const - it is saying to the client of your code, don't change it. If changes are to be allowed, then a non-const version is also required.

The wrapper class you have is there to manage the resource, so I would advise returning a reference (hence for observation) over a pointer; a pointer can be a nullptr so if there is an "optional" nature to the value, a nullptr pointer can be used to indicate that.

If the shared and const-ness of the return type is to be maintained, David Hammen's solution of shared_ptr<const lib_type> is neat.