I am working with a legacy C API under which acquiring some resource is expensive and freeing that resource is absolutely critical. I am using C++14 and I want to create a class to manage these resources. Here is the basic skeleton of the thing...
class Thing
{
private:
void* _legacy;
public:
void Operation1(...);
int Operation2(...);
string Operation3(...);
private:
Thing(void* legacy) :
_legacy(legacy)
{
}
};
This is not really the singleton pattern. Nothing is static and there may be many Thing instance, all managing their own legacy resources. Furthermore, this is not merely a smart-pointer. The wrapped pointer, _legacy is private and all operations are exposed via some public instance functions that hide the legacy API from consumers.
The constructor is private because instances of Thing will be returned from a static factory or named-constructor that will actually acquire the resource. Here is a cheap imitation of that factory, using malloc() as a place-holder for the code that would invoke the legacy API ...
public:
static Thing Acquire()
{
// Do many things to acquire the thing via the legacy API
void* legacy = malloc(16);
// Return a constructed thing
return Thing(legacy);
}
Here is the destructor which is responsible for freeing the legacy resource, again, free() is just a placeholder ...
~Thing() noexcept
{
if (nullptr != _legacy)
{
// Do many things to free the thing via the legacy API
// (BUT do not throw any exceptions!)
free(_legacy);
_legacy = nullptr;
}
}
Now, I want to ensure that exactly one legacy resource is managed by exactly one instance of Thing. I did not want consumers of the Thing class to pass instances around at will - they must either be owned locally to the class or function, either directly or via unique_ptr, or wrapped with a shared_ptr that can be passed about. To this end, I deleted the assignment operator and copy constructors...
private:
Thing(Thing const&) = delete;
void operator=(Thing const&) = delete;
However, this added an additional challenge. Either I had to change my factory method to return a unique_ptr<Thing> or a shared_ptr<Thing> or I had to implement move semantics. I did not want to dictate the pattern under which Thing should be used so I chose to add a move-constructor and move-assignment-operator as follows...
Thing(Thing&& old) noexcept : _legacy(old._legacy)
{
// Reset the old thing's state to reflect the move
old._legacy = nullptr;
}
Thing& operator= (Thing&& old) noexcept
{
if (&old != this)
{
swap(_legacy, old._legacy);
}
return (*this);
}
With this all done, I could use Thing as a local and move it about...
Thing one = Thing::Acquire();
Thing two = move(one);
I could not break the pattern by attempting to commit self-assignment:
Thing one = Thing::Acquire();
one = one; // Build error!
I could also make a unique_ptr to one...
auto three = make_unique<Thing>(Thing::Acquire());
Or a shared_ptr ...
auto three = make_shared<Thing>(Thing::Acquire());
Everything worked as I had expected and my destructor ran at exactly the right moment in all my tests. In fact, the only irritation was that make_unique and make_shared both actually invoked the move-constructor - it wasn't optimized away like I had hoped.
First Question: Have I implemented the move-constructor and move-assignment-operator correctly? (They're fairly new to me and this will be the first time I have used one in anger.)
Second Question: Please comment on this pattern! Is this a good way to wrap legacy resources in a C++14 class?
Finally: Should I change anything to make the code better, faster, simpler or more readable?
struct free_thing{
void operator()(void* p)const{
// stuff
free(p);
}
};
using upthing=std::unique_ptr<void,free_thing>;
upthing make_thing(stuff){
void*retval;
// code
return upthing(retval);
}
Store a upthing in your Thing as _legacy. Use default dtor, move ctor, move assign for Thing (=default).
The destroy code goes in free_thing.
Your ctor creates the upthing.
Now users can treat your Thing as a move only value type.
Do not write your own pointer managers unless you really need to. Unique and shared do lots for you: if you write your own smart pointer, use them as internal guts even.