I'm having a debate with a co-worker about throwing exceptions from constructors, and thought I would like some feedback.
Is it OK to throw exceptions from constructors, from a design point of view?
Lets say I'm wrapping a POSIX mutex in a class, it would look something like this:
class Mutex {
public:
Mutex() {
if (pthread_mutex_init(&mutex_, 0) != 0) {
throw MutexInitException();
}
}
~Mutex() {
pthread_mutex_destroy(&mutex_);
}
void lock() {
if (pthread_mutex_lock(&mutex_) != 0) {
throw MutexLockException();
}
}
void unlock() {
if (pthread_mutex_unlock(&mutex_) != 0) {
throw MutexUnlockException();
}
}
private:
pthread_mutex_t mutex_;
};
My question is, is this the standard way to do it? Because if the pthread mutex_init call fails the mutex object is unusable so throwing an exception ensures that the mutex won't be created.
Should I rather create a member function init for the Mutex class and call pthread mutex_init within which would return a bool based on pthread mutex_init's return? This way I don't have to use exceptions for such a low level object.
This question is related to
c++
exception
constructor
throw
Apart from the fact that you do not need to throw from the constructor in your specific case because pthread_mutex_lock actually returns an EINVAL if your mutex has not been initialized and you can throw after the call to lock as is done in std::mutex:
void
lock()
{
int __e = __gthread_mutex_lock(&_M_mutex);
// EINVAL, EAGAIN, EBUSY, EINVAL, EDEADLK(may)
if (__e)
__throw_system_error(__e);
}
then in general throwing from constructors is ok for acquisition errors during construction, and in compliance with RAII ( Resource-acquisition-is-Initialization ) programming paradigm.
Check this example on RAII
void write_to_file (const std::string & message) {
// mutex to protect file access (shared across threads)
static std::mutex mutex;
// lock mutex before accessing file
std::lock_guard<std::mutex> lock(mutex);
// try to open file
std::ofstream file("example.txt");
if (!file.is_open())
throw std::runtime_error("unable to open file");
// write message to file
file << message << std::endl;
// file will be closed 1st when leaving scope (regardless of exception)
// mutex will be unlocked 2nd (from lock destructor) when leaving
// scope (regardless of exception)
}
Focus on these statements:
static std::mutex mutexstd::lock_guard<std::mutex> lock(mutex);std::ofstream file("example.txt");The first statement is RAII and noexcept. In (2) it is clear that RAII is applied on lock_guard and it actually can throw , whereas in (3) ofstream seems not to be RAII , since the objects state has to be checked by calling is_open() that checks the failbit flag.
At first glance it seems that it is undecided on what it the standard way and in the first case std::mutex does not throw in initialization , *in contrast to OP implementation * . In the second case it will throw whatever is thrown from std::mutex::lock, and in the third there is no throw at all.
Notice the differences:
(1) Can be declared static, and will actually be declared as a member variable (2) Will never actually be expected to be declared as a member variable (3) Is expected to be declared as a member variable, and the underlying resource may not always be available.
All these forms are RAII; to resolve this, one must analyse RAII.
This does not require you to initialize and connect everything on construction. For example when you would create a network client object you would not actually connect it to the server upon creation, since it is a slow operation with failures. You would instead write a connect function to do just that. On the other hand you could create the buffers or just set its state.
Therefore, your issue boils down to defining your initial state. If in your case your initial state is mutex must be initialized then you should throw from the constructor. In contrast it is just fine not to initialize then ( as is done in std::mutex ), and define your invariant state as mutex is created . At any rate the invariant is not compromized necessarily by the state of its member object, since the mutex_ object mutates between locked and unlocked through the Mutex public methods Mutex::lock() and Mutex::unlock().
class Mutex {
private:
int e;
pthread_mutex_t mutex_;
public:
Mutex(): e(0) {
e = pthread_mutex_init(&mutex_);
}
void lock() {
e = pthread_mutex_lock(&mutex_);
if( e == EINVAL )
{
throw MutexInitException();
}
else (e ) {
throw MutexLockException();
}
}
// ... the rest of your class
};
Adding to all the answers here, I thought to mention, a very specific reason/scenario where you might want to prefer to throw the exception from the class's Init method and not from the Ctor (which off course is the preferred and more common approach).
I will mention in advance that this example (scenario) assumes that you don't use "smart pointers" (i.e.- std::unique_ptr) for your class'
s pointer(s) data members.
So to the point: In case, you wish that the Dtor of your class will "take action" when you invoke it after (for this case) you catch the exception that your Init() method threw - you MUST not throw the exception from the Ctor, cause a Dtor invocation for Ctor's are NOT invoked on "half-baked" objects.
See the below example to demonstrate my point:
#include <iostream>
using namespace std;
class A
{
public:
A(int a)
: m_a(a)
{
cout << "A::A - setting m_a to:" << m_a << endl;
}
~A()
{
cout << "A::~A" << endl;
}
int m_a;
};
class B
{
public:
B(int b)
: m_b(b)
{
cout << "B::B - setting m_b to:" << m_b << endl;
}
~B()
{
cout << "B::~B" << endl;
}
int m_b;
};
class C
{
public:
C(int a, int b, const string& str)
: m_a(nullptr)
, m_b(nullptr)
, m_str(str)
{
m_a = new A(a);
cout << "C::C - setting m_a to a newly A object created on the heap (address):" << m_a << endl;
if (b == 0)
{
throw exception("sample exception to simulate situation where m_b was not fully initialized in class C ctor");
}
m_b = new B(b);
cout << "C::C - setting m_b to a newly B object created on the heap (address):" << m_b << endl;
}
~C()
{
delete m_a;
delete m_b;
cout << "C::~C" << endl;
}
A* m_a;
B* m_b;
string m_str;
};
class D
{
public:
D()
: m_a(nullptr)
, m_b(nullptr)
{
cout << "D::D" << endl;
}
void InitD(int a, int b)
{
cout << "D::InitD" << endl;
m_a = new A(a);
throw exception("sample exception to simulate situation where m_b was not fully initialized in class D Init() method");
m_b = new B(b);
}
~D()
{
delete m_a;
delete m_b;
cout << "D::~D" << endl;
}
A* m_a;
B* m_b;
};
void item10Usage()
{
cout << "item10Usage - start" << endl;
// 1) invoke a normal creation of a C object - on the stack
// Due to the fact that C's ctor throws an exception - its dtor
// won't be invoked when we leave this scope
{
try
{
C c(1, 0, "str1");
}
catch (const exception& e)
{
cout << "item10Usage - caught an exception when trying to create a C object on the stack:" << e.what() << endl;
}
}
// 2) same as in 1) for a heap based C object - the explicit call to
// C's dtor (delete pc) won't have any effect
C* pc = 0;
try
{
pc = new C(1, 0, "str2");
}
catch (const exception& e)
{
cout << "item10Usage - caught an exception while trying to create a new C object on the heap:" << e.what() << endl;
delete pc; // 2a)
}
// 3) Here, on the other hand, the call to delete pd will indeed
// invoke D's dtor
D* pd = new D();
try
{
pd->InitD(1,0);
}
catch (const exception& e)
{
cout << "item10Usage - caught an exception while trying to init a D object:" << e.what() << endl;
delete pd;
}
cout << "\n \n item10Usage - end" << endl;
}
int main(int argc, char** argv)
{
cout << "main - start" << endl;
item10Usage();
cout << "\n \n main - end" << endl;
return 0;
}
I will mention again, that it is not the recommended approach, just wanted to share an additional point of view.
Also, as you might have seen from some of the print in the code - it is based on item 10 in the fantastic "More effective C++" by Scott Meyers (1st edition).
It is OK to throw from your constructor, but you should make sure that your object is constructed after main has started and before it finishes:
class A
{
public:
A () {
throw int ();
}
};
A a; // Implementation defined behaviour if exception is thrown (15.3/13)
int main ()
{
try
{
// Exception for 'a' not caught here.
}
catch (int)
{
}
}
Throwing an exception is the best way of dealing with constructor failure. You should particularly avoid half-constructing an object and then relying on users of your class to detect construction failure by testing flag variables of some sort.
On a related point, the fact that you have several different exception types for dealing with mutex errors worries me slightly. Inheritance is a great tool, but it can be over-used. In this case I would probably prefer a single MutexError exception, possibly containing an informative error message.
#include <iostream>
class bar
{
public:
bar()
{
std::cout << "bar() called" << std::endl;
}
~bar()
{
std::cout << "~bar() called" << std::endl;
}
};
class foo
{
public:
foo()
: b(new bar())
{
std::cout << "foo() called" << std::endl;
throw "throw something";
}
~foo()
{
delete b;
std::cout << "~foo() called" << std::endl;
}
private:
bar *b;
};
int main(void)
{
try {
std::cout << "heap: new foo" << std::endl;
foo *f = new foo();
} catch (const char *e) {
std::cout << "heap exception: " << e << std::endl;
}
try {
std::cout << "stack: foo" << std::endl;
foo f;
} catch (const char *e) {
std::cout << "stack exception: " << e << std::endl;
}
return 0;
}
the output:
heap: new foo
bar() called
foo() called
heap exception: throw something
stack: foo
bar() called
foo() called
stack exception: throw something
the destructors are not called, so if a exception need to be thrown in a constructor, a lot of stuff(e.g. clean up?) to do.
If your project generally relies on exceptions to distinguish bad data from good data, then throwing an exception from the constructor is better solution than not throwing. If exception is not thrown, then object is initialized in a zombie state. Such object needs to expose a flag which says whether the object is correct or not. Something like this:
class Scaler
{
public:
Scaler(double factor)
{
if (factor == 0)
{
_state = 0;
}
else
{
_state = 1;
_factor = factor;
}
}
double ScaleMe(double value)
{
if (!_state)
throw "Invalid object state.";
return value / _factor;
}
int IsValid()
{
return _status;
}
private:
double _factor;
int _state;
}
Problem with this approach is on the caller side. Every user of the class would have to do an if before actually using the object. This is a call for bugs - there's nothing simpler than forgetting to test a condition before continuing.
In case of throwing an exception from the constructor, entity which constructs the object is supposed to take care of problems immediately. Object consumers down the stream are free to assume that object is 100% operational from the mere fact that they obtained it.
This discussion can continue in many directions.
For example, using exceptions as a matter of validation is a bad practice. One way to do it is a Try pattern in conjunction with factory class. If you're already using factories, then write two methods:
class ScalerFactory
{
public:
Scaler CreateScaler(double factor) { ... }
int TryCreateScaler(double factor, Scaler **scaler) { ... };
}
With this solution you can obtain the status flag in-place, as a return value of the factory method, without ever entering the constructor with bad data.
Second thing is if you are covering the code with automated tests. In that case every piece of code which uses object which does not throw exceptions would have to be covered with one additional test - whether it acts correctly when IsValid() method returns false. This explains quite well that initializing objects in zombie state is a bad idea.
If you do throw an exception from a constructor, keep in mind that you need to use the function try/catch syntax if you need to catch that exception in a constructor initializer list.
e.g.
func::func() : foo()
{
try {...}
catch (...) // will NOT catch exceptions thrown from foo constructor
{ ... }
}
vs.
func::func()
try : foo() {...}
catch (...) // will catch exceptions thrown from foo constructor
{ ... }
The only time you would NOT throw exceptions from constructors is if your project has a rule against using exceptions (for instance, Google doesn't like exceptions). In that case, you wouldn't want to use exceptions in your constructor any more than anywhere else, and you'd have to have an init method of some sort instead.
Source: Stackoverflow.com