3

My class contains field:

private:
    OrderUpdate curOrderUpdate;

I'm using it over and over and it often need to be reinitialized:

for (int i = 0; i < entries.size(); i++) {
    auto entry = entries[i];
    new (&curOrderUpdate) OrderUpdate();
    curOrderUpdate.MDEntryID = entry.get_MDEntryID().value()[0];
            ...

I have several questions:

  • can I use variable or I must change it to a pointer? Change OrderUpdate curOrderUpdate to OrderUpdate* curOrderUpdate?
  • is assign mandatory? Should I write curOrderUpdate = new (&curOrderUpdate) OrderUpdate(); or just new (&curOrderUpdate) OrderUpdate() is enough?
Oleg Vazhnev
  • 23,239
  • 54
  • 171
  • 305

2 Answers2

3

To answer your specific questions:

  • Yes, you can store the object directly (not via pointer), so OrderUpdae curOrderUpdate is fine.

  • No, you don't have to (actually you shouldn't) assign. Invoking the constructor in situ is correct: `new (&curOrderUpdate) OrderUpdate()

However, to make your code safe, you must invoke the destructor before re-invoking the constructor. So you would change the loop like this:

for (int i = 0; i < entries.size(); i++) {
    auto entry = entries[i];
    curOrderUpdate.~OrderUpdate();
    new (&curOrderUpdate) OrderUpdate();
    curOrderUpdate.MDEntryID = entry.get_MDEntryID().value()[0];

However, such low-level code is best reserved for very special cases like unions and union-like structures. In particular, as @JamesKanze points out in the comments, the code has a serious problem if the constructor throws (and the destructor is not trivial) - your object would then be destructed twice.

It would be much cleaner design if you simply added a reinitialise() function to OrderUpdate() which would do the right thing without resorting to destructor and constructor invocation.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • 1
    A `reinitialize()` is is entirely the correct approach. The constructor cannot throw an exception in the example as-written. If it does, the surrounding scope has no way of knowing `curOrderUpdate` has been destroyed and you find yourself with a double-destruction when unwinding. – WhozCraig Nov 28 '13 at 11:05
  • It's just the opposite. If the destructor is trivial, the code is well defined (but not something I'd like to have to maintain) without calling the destructor. If you call the destructor, however, and the placement new fails for some reason, you have undefined behavior; using destructor followed by placement new to replace assignment is _never_ a good idea. – James Kanze Nov 28 '13 at 11:16
  • @JamesKanze Sorry, James. Was slightly confused. Is your comment in reference to the answer or to my comment (or both?) ? Late here. probably should be in bed right now, so I apologize in advance for the potentially obvious query. – WhozCraig Nov 28 '13 at 11:22
  • Your answer. Destructing, then using placement new, means that there is an interval in which the object is in an invalid state. If an exception occurs at that moment (e.g. in the constructor invoked by placement new), the destructor will be called on an object in an invalid state, resulting in undefined behavior. – James Kanze Nov 28 '13 at 11:48
  • @JamesKanze Heh. I did not have an answer (mine was the comment), but your comment seems to reenforce my comment (I *think*). so I seem to have understood. – WhozCraig Nov 28 '13 at 13:34
1

It's dubious to call a constructor repeatedly on the same object. The correct way to do this is

curOrderUpdate = OrderUpdate();
john
  • 85,011
  • 4
  • 57
  • 81
  • will this allocate new object? – Oleg Vazhnev Nov 28 '13 at 10:59
  • 1
    It creates a temporary OrderUpdate object, assigns it to curOrderUpdate, then destroys the temporary object. If you want to avoid the creation of a temporary object then I think the best way would be to add an `clear` method to OrderUpdate. This method can do the reinitialisation. – john Nov 28 '13 at 11:00
  • It's an error in what sense? There's certainly nothing wrong syntactically with using placement new on an existing object, and the behavior is well defined _if_ the object has a trivial destructor. (Whether it is good practice is another issue.) – James Kanze Nov 28 '13 at 11:14
  • @JamesKanze That surprises me. – john Nov 28 '13 at 11:19
  • @john It's in §3.8: "The lifetime of an object of type T ends when: [...]-- the storaage which the object occupies is reused or released." Using placement new on the object is reuse. (Note that I do _not_ think that this is a good programming technique. Just that it is formally legal.) – James Kanze Nov 28 '13 at 11:46
  • Did this change with C++11? I thought I remembered that repeatedly constructing an object (without an intervening destruction) was an error, but maybe not. – john Nov 28 '13 at 11:59