Selecting between two constructors

Question

Problem: I have a non-copyable object with two constructors. I need to create an object with one of the constructors and then use it within some common code:-

With a copyable object it would look like this, and be easy:

Object a;

if (condition) 
   a = Object(p1);
else
   a = Object(p2,p3,p4);

a.doSomething();

But, the object is non-copyable, so I've had to do this:

boost::scoped_ptr<Object> a;

if (condition) 
   a = new Object(p1);
else
   a = new Object(p2,p3,p4);

a->doSomething();

This feels too complex. Is there a better solutiuon?

Answer 1

Here's a very terrible hack, assuming Object is default-constructible:

Object a;
a.~Object();

if (condition) { ::new (&a) Object(p1); }
else           { ::new (&a) Object(p2, p3, p4); }

Don't use this.

Another option is using a union, but you'll need to invoke the destructor manually in that setup as well.

---

A cleaner solution could be achieved with Boost.Optional (using in-place factories). (Thanks to @K-Ballo for digging up the details!)

#include <boost/optional.hpp>
#include <boost/utility/in_place_factory.hpp>

struct Object
{
    explicit Object(int) {}
    explicit Object(int, float, std::string) {}

    Object(Object const &)             = delete;
    Object(Object &&)                  = delete;
    Object & operator=(Object const &) = delete;
    Object & operator=(Object &&)      = delete;
};

boost::optional<Object> a;

if (condition) { a = boost::in_place(0); }
else           { a = boost::in_place(0, 1.0f, "two" ); }

Answer 2

Looks perfectly reasonable to me just the way it is. It's clear, simple and relatively concise.

Answer 3

I don't see the complexity... If you need to construct efficiently based on an if-condition declaring a pointer and using new is your only option. What you don't necessarily need to do is:

1. Use a scoped_ptr (altough that's usually a good idea)
2. Have the constructor in the if in your "main" code. Yours is a typical use case for a factory (see e.g. http://en.wikipedia.org/wiki/Factory_method_pattern).

EDIT: added "efficiently" in the second sentence.

Answer 4

auto const doSomethingTo = []( Object&& o ) { o.doSomething(); };
doSomethingTo( condition? Object( p1 ) : Object( p1, p2, p3 ) );

Disclaimer: code not touched by compiler.

---

EDIT: the code above, when the Object( Object&& ) constructor is private, fails to compile with MSVC 11.0 (yes even last year's November CTP), but does compile fine with MinGW g++ 4.7.1 and with some version of clang.

It appears that it should compile.

So, it's probably a bug in Visual C++ – but unfortunately I didn't find an easy workaround.

---

An uneasy workaround for the assumed-to-be Visual C++ compiler bug:

#include <fstream>
#include <iostream>
using namespace std;

class Object
{
private:
    Object( Object const& );
    Object( Object&& );
public:
    void doSomething() const {}
    Object( int ) {}
    Object( int, int, int ) {}
};

int main( int argc, char* argv[] )
{
    int p1 = 0, p2 = 0, p3 = 0;
    bool condition = argc == 2;

    auto const doSomething1 = [=]() { Object o( p1 ); o.doSomething(); };
    auto const doSomething2 = [=]() { Object o( p1, p2, p3 ); o.doSomething(); };

    if( condition ) { doSomething1(); } else { doSomething2(); }
}

Another answer maintains that new (read: a dynamic allocation) is your only option.

That's wrong.

Answer 5

There's really nothing wrong with your solution, although as others have already mentionned, it would be more readably if you used the conditional operator rather than if's. But you should consider the possibility of refactoring. If you factor out all of the code which uses the object into a separate function (taking the object by reference), then something like:

if ( condition ) {
    Object a( p1 );
    doWhatever( a );
} else {
    Object a( p2, p3, p4 );
    doWhatever( a );
}

might be preferable (or not—I don't think that there's any "right" answer with regards to choosing between these two).

Answer 6

I think your code is OK.

You just may want to consider the conditional operator ? :

boost::scoped_ptr<Object> a( condition ? new Object(p1) : new Object(p2,p3,p4) );
a->doSomething();

Answer 7

So, here is a quick trick to make this work, without manually constructing objects. Instead, I created a Deferred<T> template that represents an object in automatic storage whose construction is deferred (and possibly never occurs).

The buff in Deferred should be replaced with a union, because that will handle alignment issues (assuming you have C++11 features to support that). Asserts that constructed is true when you call get() is probably a good idea.

Once you have constructed your object, you can implicitly cast your Deferred<T> to a T&, then use that T& as an alias to the deferred-constructed T.

In theory, you could do away with the constructed bool if you could prove that it would always be constructed, but I'd advise against it. Other than that, this should be nearly as efficient as you can pull it off. And with the C++11 union case, it might even be standards compliant.

Oh yes, and it should be enhanced with perfect forwarding.

#include <utility>

// does not handle alignment issues:
template<typename T>
struct Deferred {
   Deferred():constructed(false) {}
  operator T&() { return get(); }
  T& get() { return *reinterpret_cast<T*>(&buff[0]); }
  template<typename... Args>
  T& construct( Args... args ) {
    new(&buff[0]) T(args...);
    constructed = true;
    return get();
  }
  ~Deferred() {
    if (constructed) {
      get().~T();
    }
  }
private:
  bool constructed;
  char buff[sizeof(T)];
};

#include <iostream>

struct Object {
  bool is_int;
  Object( int x ):is_int(true) {}
  Object( double d ):is_int(false) {}
  ~Object() {
    std::cout << "~Object("<<is_int<<") destroyed\n";
  }
};

enum which_test {
  as_int,
  as_double,
  do_not,
};
void test(which_test v) {
  std::cout << v << "\n";
  Deferred<Object> o;
  if(v==as_int) {
    o.construct( 7 );
  } else if (v==as_double) {
    o.construct( 7.0 );
  } else {
  }
}

int main() {
  test(as_int);
  test(as_double);
  test(do_not);
}