views:

73

answers:

1

Suppose I have a class that requires copy constructor to be called to make a correct copy of:

struct CWeird
{
    CWeird() { number = 47; target = &number; }

    CWeird(const CWeird &other) : number(other.number), target(&number) { }

    const CWeird& operator=(const CWeird &w) { number = w.number; return *this; }

    void output()
    {
        printf("%d %d\n", *target, number);
    }

    int *target, number;
};

Now the trouble is that CArray doesn't call copy constructors on its elements when reallocating memory (only memcpy from the old memory to the new), e.g. this code

CArray<CWeird> a;
a.SetSize(1);
a[0].output();

a.SetSize(2);
a[0].output();

results in

47 47
-572662307 47

I don't get this. Why is it that std::vector can copy the same objects properly and CArray can't? What's the lesson here? Should I use only classes that don't require explicit copy constructors? Or is it a bad idea to use CArray for anything serious?

+3  A: 

The copied pointer still points to the original number, which no longer exists, since the array has been reallocated due to the resize.

I'm guessing that CArray uses assignment rather than copy-construction. Define an assignment operator to see if this fixes it:

CWeird& operator=(CWeird w) { swap(w); return *this; }
void swap(CWeird& w) { int t = number; number = w.number; w.number = t; }

It's generally a good idea to do this anyway, to avoid inconsistent behaviour between copy-construction and assignment.

FYI, the above code uses an idiomatic approach to implementing assignment semantics with strong exception-safety guarantees:

  1. Returning a non-const reference is very much the standard for operator=, since it matches the semantics of primitive types.
  2. Passing the parameter by value is the easiest way to make a copy of the original, and guarantees that this object won't be affected if the copy constructor fails.
  3. The call to swap switches the passed-in copy with this object in a way that will never throw an exception, thus effecting the assignment in a completely exception-safe manner.

In this case, it would be simpler to just assign the number, but I habitually implement all my assignments this way to avoid being caught with my pants down if a future maintainer makes it possible for copying to throw an exception.

Marcelo Cantos
I don't mind down-votes, but please say why.
Marcelo Cantos
I'm gonna upvote.
DeadMG
Kind thanks, @DeadMG. It seems that the down-voter had a change of heart, anyway.
Marcelo Cantos
Thanks, I forgot about the assignment operator. However it doesn't change anything since CArray copies the memory like this (from afxtempl.h):// copy new data from old::ATL::Checked::memcpy_s(pNewData, (size_t)nNewMax * sizeof(TYPE), m_pData, (size_t)m_nSize * sizeof(TYPE));
MMx
MMx
@MMx, that's an alarmingly stupid implementation of array copying. On the strength of that alone, I wouldn't touch CArray with a barge-pole.
Marcelo Cantos
@MMx, I've amended the answer to cover the questions in your comments.
Marcelo Cantos
Well, it's called CArray, so I guess it's for C types.
DeadMG
@DeadMG, The `C` prefix is used all throughout MFC to denote "class". (I'm not sure if you were being ironic, so I apologise if I spoiled the joke.)
Marcelo Cantos
You did indeed spoil the joke. Made me cry.
DeadMG