Pregunta

Real example is obviously much longer, but this sums up my problem:

class Object
{
 int mInt1,mInt2;
 Object::Object();
 Object::Object(int param1);
 Object::Object(int param1, int param2);
};
Object::Object(){}
Object::Object(int param1):mInt1(param1){}
Object::Object(int param1, int param2):mInt1(param1),mInt1(param2){}

Then in main:

if (type1){
  Object instance(param1);
}
else{
  Object instance(param1,param2);
}
// do stuff with instance

Whoops! That won't work, instance is out of scope for the program that follows.

Object instance;
if (type1){
  instance = Object(param1);
}
else{
  instance = Object(param1,param2);
}
// do stuff with instance

But now I run in to trouble because I didn't have a copy constructor defined. I'd really rather not write a copy-constructor because my actual class has dozens of members, many of which are non-basic types and might require even more work to copy.

Specifically, I am getting

main.cpp: error: use of deleted function ‘Object& Object::operator=(Object&&)’
         instance = Object(param1);
                  ^
note: ‘Object& Object::operator=(Object&&)’ is implicitly deleted because the default definition would be ill-formed:
¿Fue útil?

Solución

The universal way to deal with non-copyable objects is to throw it into a unique_ptr (or auto_ptr, depending on your compiler).

  std::unique_ptr<Object> instance;

  if (type1) {
    instance.reset(new Object(i));
  }
  else {
    instance.reset(new Object(i, j));
  }

Using raw pointers here really isn't safe because once you start having to deal with exceptions or any interesting code paths it becomes a chore to worry about leaks. Trust me, in 100% of cases, you will have less work and lines of code to deal with if you just drop it in a unique_ptr.


An optimal solution would be to redesign Object's constructors, because circumventing non-copyability may leave the object in an illegal state. In general, you want to preserve non-copyability if the compiler thinks it's necessary. We don't have the details here to flesh out such a solution however.

Otros consejos

If you do not want dynamically allocation then you can use an Initialize function:

class Object
{
 int mInt1,mInt2;
 Object::Object();
 Object::Initialize();
 Object::Initialize(int param1);
 Object::Initialize(int param1, int param2);
};
Object::Object(){Initialize();} //call the empty Initialize for nice coding...:)
Object::Initialize(){  }
Object::Initialize(int param1){ mInt1(param1); }
Object::Initialize(int param1, int param2){ mInt1(param1);mInt1(param2);}

Then you can use initialize to select the type.

Object instance;
if (type1){
  instance.Initialize(param1);
}
else{
  instance.Initialize(param1,param2);
}

Here is a low-level(ish) solution which does what you want. I will leave it up to you to decide whether it's a good idea to use it:

#include <type_traits>

template <class T>
class MultiInitialiser
{
  T *object;
  std::aligned_storage<T> storage;

public:
  MultiInitialiser() : object(nullptr)
  {}

  template <class... Arg>
  void initialise(Arg &&... arg)
  {
    if (object)
      throw "Double init error";
    object = new (&storage) T(std::forward<Arg>(arg)...);
  }

  operator T& ()
  { return *object; }

  operator const T& () const
  { return *object; }

  ~MultiInitialiser()
  {
    if (object)
      object->~T();
  }
};

Copy/move operations for the above are left as an excercise for the reader ;-)

The class would then be used like this:

MultiInitialiser<Object> instance;
if (type1){
  instance.initialise(param1);
}
else{
  instance.initialise(param1,param2);
}

Apart from the cast to T, you could also give the class operator* and operator-> returning the contained object, similar to what boost::optional does.

There are a couple of options that let you do this while retaining automatic storage, and which one you should use depends on the semantics of the type Object.

The POD

If you have a type like the one given in the question, you might choose to reduce it to a POD type; basically, remove all of the user-provided constructors and give everything the same access specifier:

struct Object {
    int mInt1, mInt2;
};

Then, your initialization pattern might look like this (using placement new):

Object o; // Default-initialized, a no-op

if (condition)
    new (&o) Object {i};
else
    new (&o) Object {i, j};

General Types

Generally speaking, your typical value-semantic type will work perfectly fine if you default-initialize and then assign, thanks to move semantics:

std::vector <foo> v;

if (condition)
    v = std::vector <foo> (42);
else
    v = std::vector <foo> {bar, baz, quux};

Often, though, you'll still be doing work in the default constructor, because certain types' default-constructed objects (e.g., std::vector) have well-defined state. If you want to avoid this work for an arbitrary predefined type, you might want to use std::optional (as of this writing not actually yet standard):

std::optional <big_but_flat> b;

if (condition)
    b.emplace (i);
else
    b.emplace (i, j);

Without std::optional

You might object that std::optional has too much overhead associated with it, and I'll leave it to you and your measurements to decide whether that's the case. At any rate, we can get our behaviour without worrying about that overhead— but may the nasal demons have mercy if you don't actually perform your initialization. We'll use a union to get what we want:

// At function scope
union store_v {
    std::vector <int> v;

    store_v () {}
    ~store_v () { v.~vector <int> (); }
} sv;

if (condition)
    new (&sv.v) std::vector <int> (42);
else
    new (&sv.v) std::vector <int> {49, 343, 2401};

This may be improved. For example, we can make the storage a template:

template <typename T>
union store {
    T t;

    store () {}
    ~store () { t.~T (); }
};

// At function scope
store <std::vector <int>> sv;
if (condition)
    new (&sv.t) std::vector <int> (42);
else
    new (&sv.t) std::vector <int> {49, 343, 2401};

We can give ourselves a reference:

template <typename T>
union store {
    T t;

    store () {}
    ~store () { t.~T (); }
};

// At function scope
store <std::vector <int>> sv;
auto&& v = sv.t; // Deduce const, for what that's worth

if (condition)
    new (&v) std::vector <int> (42);
else
    new (&v) std::vector <int> {49, 343, 2401};

And with a little attention to detail to avoid name collisions and handle C++'s… interesting declaration syntax, we could even define a couple of macros to clean the code up (implementation left as an exercise to the reader):

template <typename T>
union store {
    T t;

    store () {}
    ~store () { t.~T (); }
};

// At function scope
DECL_UNINIT (std::vector <int>, v);

if (condition)
    INIT (v, (42));
else
    INIT (v, {49, 343, 2401});

You can use a pointer to your object and instantiate it by new operator:

Object * instance;
if (type1){
  instance = new Object(param1);
}
else{
  instance = new Object(param1,param2);
} 

You're using something that is called copy elision.

This states that the compiler MAY optimize the code and avoid a copy constructor in such a case. But it doesn't have to, and may use a copy constructor anyway.

The right way (without being subjected to the whims of the compiler) would be to use a pointer:

Object* instance;
if (type1){
  instance = new Object(param1);
}
else{
  instance = new Object(param1,param2);
}

In your version of one-parameter constructor, the mInt2 member is just ignored (isn't ever initialized), so I assume that you don't do any computation with that member if type1 is false (though I don't know how you're doing it without storing type1).

So, why don't just change the dessign? Make te constructor take int param1, int param2 and type1 as parameters and choose internally how to build himself:

class Object
{
 int mInt1,mInt2;

 Object::Object() :
   mInt1(0), // don't forget to initialize your values!
   mInt2(0)
 {}

 // Object::Object(int param1); no more 1-parameter ctor.

 Object::Object(int param1, int param2, type type1) :
   mInt1(param1),
   mInt2(type1 ? param2 : 0) // assuming that 0 isn't a valid value for mInt2
 {}
};

Then in main:

Object instance(param1, param2, type1);
// do stuff with instance

I guess that it looks a little neater.

You could write a move assignment. Depending what your data members look like, you may get away with memcopying some or all of them, cf. Move constructor with memcpy.

That said, I assume that either you need a full set of constructors/destructors, including copy and assignment; that will always be necessary if you want to have it in containers, assign it etc. Or else the class doesn't need any of that and you just initialize the needed parts of it depending on the situation, and when you are done, you de-initialize manually.

I think I found a way to do it without requiring allocating the object on the heap, using a lambda. Basically, the idea is to separate the construction from the usage. (the following assume that you fixed the difinition of Object so that it compiles)

auto work = []( Object& object ){ 
   // here do the work on the object
};

// now create the object and apply the work in the process:
if (type1){
  Object instance(param1);
  work( instance );
}
else{
  Object instance(param1,param2);
  work( instance );
}

Here no copy was involved but the same code is still applied whatever the wy the object was constructed, and without having to declare an external function (as a lambda is a local function). From the memory point of view, there will always be only one instance object so the stack memory allocated will always be the same size whatever the path.

Obviously, this don't work if the instance have to get out of the scope of the whole function. If it's the case, then you really need to allocate it on the heap using a smart pointer preferably.

Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top