Error handling is a challenge in C++ constructors. There are several common approaches but all of them has obvious disadvantages. Throwing exceptions for example, may cause leak of the allocated resources earlier in the constructor, making it an error prone approach. Using a static init() method is another common solution, but it goes against the RAII principle.
Studying the subject I found this answer and blog suggesting the use of C++17 feature named std::optional<>, and I found it promising. However it seems that this kind of solution comes with an underlying problem - it triggers the destructor instantly when the user retrieved the object.
Here is a simple code example describing the problem, my code is based on the the above sources
class A
{
public:
A(int myNum);
~A();
static std::optional<A> make(int myNum);
bool isBuf() { return _buf; };
private:
char* _buf;
};
std::optional<A> A::make(int myNum)
{
std::cout << "A::make()\n";
if (myNum < 8)
return {};
return A(myNum);
}
A::A(int myNum)
{
std::cout << "A()\n";
_buf = new char[myNum];
}
A::~A()
{
std::cout << "~A()\n";
delete[]_buf;
}
int main()
{
if (std::optional<A> a = A::make(42))
{
if (a->isBuf())
std::cout << "OK\n";
else
std::cout << "NOT OK\n";
std::cout << "if() finished\n";
}
std::cout << "main finished\n";
}
The output of this program will be:
A::make()
A()
~A()
OK
if() finished
~A()
followed with a runtime error (at least in Visual C++ environment) for attempting to delete a->_buf twice.
I used cout for the reader's convenience as I found this problem debugging a much complex code, but the problem is clear - the return statement in A::make() constructs the objects, but since it is the end of the A::make() scope - the destructor is invoked. The user sure his object is initialized (notice how we got an "OK" message) while in reality it was destroyed, and when we step out of the if() scope in main, a->~A() is invoked once again.
So, am I doing this wrong?
The use of std::optional for error handling in constructors is common, or so I've been told. Thanks in advance
Your class violates the rule of 3/5.
Instrument the copy constructor and simplify main to get this:
#include <optional>
#include <iostream>
class A
{
public:
A(int myNum);
~A();
A(const A& other){
std::cout << "COPY!\n";
}
static std::optional<A> make(int myNum);
bool isBuf() { return _buf; };
private:
char* _buf = nullptr;
};
std::optional<A> A::make(int myNum)
{
std::cout << "A::make()\n";
if (myNum < 8)
return {};
return A(myNum);
}
A::A(int myNum)
{
std::cout << "A()\n";
_buf = new char[myNum];
}
A::~A()
{
std::cout << "~A()\n";
delete[]_buf;
}
int main()
{
std::optional<A> a = A::make(42);
std::cout << "main finished\n";
}
Output is:
A::make()
A()
COPY!
~A()
main finished
~A()
When you call A::make() the local A(myNum) is copied to the retunred optional and its destructor is called afterwards. You'd have the same issue without std::optional (eg by returning an A by value).
The copy constructor I added does not copy anything, but the compiler generated one does make a shallow copy of the char* _buf; member. As you do not properly deep copy the buffer it gets deleted twice which results in the runtime error.
Use a std::vector for the rule of 0, or properly implement the rule of 3/5. Your code invokes undefined behavior.
PS Not directly related to the problem, but you should initialize members instead of assigning to them in the constructors body. Change:
A::A(int myNum)
{
std::cout << "A()\n";
_buf = new char[myNum];
}
to
A::A(int myNum) : _buf( new char[myNum])
{
std::cout << "A()\n";
}
or better yet, use a std::vector as mentioned above.
PPS:
Throwing exceptions for example, may cause leak of the allocated resources earlier in the constructor, making it an error prone approach.
No, throwing from a constructor is common and has no problem when you don't manage memory via raw pointers. Both using a std::vector or a smart pointer would help to make your constructor excpetion safe.