Arguments against "initialize()" method instead of constructors

Question

I'm currently in charge of finding all bad practices in our code base and to convince my colleagues to fix the offending code. During my spelunking, I noticed that many people here use the following pattern:

class Foo
{
  public:
    Foo() { /* Do nothing here */ }
    bool initialize() { /* Do all the initialization stuff and return true on success. */ }
    ~Foo() { /* Do all the cleanup */ }
};

Now I might be wrong, but to me this initialize() method thing is awful. I believe it cancels the whole purpose of having constructors.

When I ask my collegues why this design decision was made, they always answer that they have no choice because you can't exit a constructor without throwing (I guess they assume throwing is always bad).

I failed to convince them so far and I admit I may lack of valuable arguments... so here is my question: Am I right that this construct is a pain and if so, what issues do you see in it ?

Thank you.

Answer 1

Both single step (constructor) initialisation and two step (with an init method) initialisation are useful patterns. Personally I feel that excluding either is a mistake, although if your conventions prohibit use of exceptions entirely then you prohibit single step initialisation for constructors that can fail.

In general I prefer single step initialisation because this means that your objects can have stronger invariants. I only use two step initialisation when I consider it meaningful or useful for an object to be able to exist in an "uninitialised" state.

With two step initialisation it is valid for your object to be in an uninitialised state - so every method that works with the object needs to be aware of and correctly handle the fact that it might be in an uninitialised state. This is analogous to working with pointers, where it is poor form to assume that a pointer is not NULL. Conversely, if you do all your initialisation in your constructor and fail with exceptions than you can add 'the object is always initialised' to your list of invariants, and so it becomes easier and safer to make assumptions about the state of the object.

Answer 2

This is usually known as Two phase or Multiphase Initialization and it is particularly bad because once a constructor call has finished successfully, you should have a ready to use object, In this case you won't have a ready-to-use object.

I cannot help but stress more on the following:
Throwing an exception from constructor in case of failure is the best and the only concise way of handling object construction failures.

Answer 3

It depends on the semantics of your object. If the initialization is something that's crucial to the data structure of the class itself, then a failure would be better handled by either throwing an exception from the constructor (e.g. if you are out of memory), or by an assertion (if you know that your code should not actually fail, ever).

On the other hand, if success or otherwise of the construction depends on user input, then failure is not an exceptional condition, but rather part of the normal, expected runtime behaviour that you need to test for. In that case, you should have a default constructor that creates an object in an "invalid" state, and an initialization function that can be called either in a constructor or later and that may or may not succeed. Take std::ifstream as an example.

So a skeleton of your class could look like this:

class Foo
{
    bool valid;
    bool initialize(Args... args) { /* ... */ }

public:
    Foo() : valid(false) { }
    Foo(Args... args) : valid (false) { valid = initialize(args...); }

    bool reset(Args... args)   // atomic, doesn't change *this on failure
    {
        Foo other(args...);
        if (other) { using std::swap; swap(*this, other); return true; }
        return false;
    }

    explicit operator bool() const { return valid; }
};