I'm coming back to C++ from a heavy C# background and I've inherited some C++ codebase which I think might not have been in line with the best C++ practices.
For example, I'm dealing with the following case (simplified):
// resource
class Resource {
HANDLE _resource = NULL;
// copying not allowed
Resource(const Resource&);
Resource& operator=(const Resource& other);
public:
Resource(std::string name) {
_resource = ::GetResource(name); if (NULL == _resource) throw "Error"; }
~Resource() {
if (_resource != NULL) { CloseHandle(_resource); _resource = NULL; };
}
operator HANDLE() const { return _resource; }
};
// resource consumer
class ResourceConsumer {
Resource _resource;
// ...
public:
void Initialize(std::string name) {
// initialize the resource
// ...
// do other things which may throw
}
}
Here ResourceConsumer
creates an instance of Resource
and does some other things. For some reason (outside my control), it exposes Initialize
method for that, rather than offering a non-default constructor, what apparently violates the RAII pattern. It's a library code and the API can't be refactored without making it a breaking change.
So my question is, how to code Initialize
correctly in this case? Is it an acceptable practice to use an in-place construction/destruction and a re-throw, like below? As I said, I came from C# where I'd simply use try/finally
or the using
pattern for that.
void ResourceConsumer::Initialize(std::string name) {
// first destroy _resource in-place
_resource.~Resource();
// then construct it in-place
new (&_resource) Resource(name);
try {
// do other things which may throw
// ...
}
catch {
// we don't want to leave _resource initialized if anything goes wrong
_resource.~Resource();
throw;
}
}
Make Resource
a moveable type. Give it move construction/assignment. Then, your Initialize
method can look like this:
void ResourceConsumer::Initialize(std::string name)
{
//Create the resource *first*.
Resource res(name);
//Move the newly-created resource into the current one.
_resource = std::move(res);
}
Note that in this example, there is no need for exception handling logic. It all works itself out. By creating the new resource first, if that creation throws an exception, then we keep the previously created resource (if any). That provides the strong exception guarantee: in the event of an exception, the state of the object is preserved exactly as it was before the exception.
And note that there is no need for explicit try
and catch
blocks. RAII just works.
Your Resource
move operations would be like this:
class Resource {
public:
Resource() = default;
Resource(std::string name) : _resource(::GetResource(name))
{
if(_resource == NULL) throw "Error";
}
Resource(Resource &&res) noexcept : _resource(res._resource)
{
res._resource = NULL;
}
Resource &operator=(Resource &&res) noexcept
{
if(&res != this)
{
reset();
_resource = res._resource;
res._resource = NULL;
}
}
~Resource()
{
reset();
}
operator HANDLE() const { return _resource; }
private:
HANDLE _resource = NULL;
void reset() noexcept
{
if (_resource != NULL)
{
CloseHandle(_resource);
_resource = NULL;
}
}
};