1

I have a struct, Foo, with a pointer array of Bar.

struct Foo
{
    Bar* _BarList;

    Foo()
    {
        _BarList = new Bar[1];
        _BarList[0] = Bar(10, 20);
    }

    Foo& operator=(const Foo& foo)
    {
        Bar* tmpBarList = new Bar[1];

        tmpBarList[0] = foo._BarList[0];

        delete[] _BarList;

        _BarList = tmpBarList;

        return *this;
    }

    ~Foo()
    {
        delete[] _BarList;
    }
};

I call it like this

Foo baz = fooFunc();

(It seems) that if I create an instance of Foo (f) inside a function (fooFunc) and return it, the destructor is called before the value is returned as I lose the contents of _BarList.

This makes sense as it was created within the function. Here is an example.

Foo fooFunc()
{
    Foo f = Foo();
    return f;
}

If I return an instance of Foo directly on the return, the destructor of that item isn't called until after the equals operator has been called (by the equals statement on the line of the call).

Foo fooFunc()
{
    return Foo();
}

I suppose this makes sense as I am creating Foo inside the object, and things are cleared up before the object is returned.

I think I can resolve this by doing the return like this (after writing a new constructor to take Foo):

Foo fooFunc()
{
    Foo f = Foo();
    return Foo(f);
}

1) Am I right in my assumptions?

2) Is there another way to do it that would not require so many repeated assignment operators being called?

Edit: Please consider that this function would normally do a lot more than just return Foo()!

Beakie
  • 1,948
  • 3
  • 20
  • 46

2 Answers2

5

Your class violates the Rule of three. You have a destructor and a copy assignment operator, but no copy constructor. And the default one most definitely does not do what you need.

And note that all of these lines:

Foo f = Foo();
return Foo(f);
Foo baz = fooFunc();

use the copy constructor, and not the assignment operator. The assignment operator is only used for assigning to an existing object, never when creating/initialising a new one.

Community
  • 1
  • 1
Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • I'm not sure `Foo f = Foo();` uses copy constructor, it uses assignment operator, compiler does the same as if you write `Foo f`. No? – jpo38 Oct 02 '14 at 08:30
  • @jpo38 `Foo f = Foo()` uses the copy constructor to copy from a temporary `Foo()`. Initialisation *always* uses a ctor, *never* an assignment op. – Angew is no longer proud of SO Oct 02 '14 at 08:57
  • My mistake, I meants to ask "are you sure `Foo f = Foo()` uses copy constructor". And the answer is yes, as posted here: http://stackoverflow.com/questions/1051379/is-there-a-difference-in-c-between-copy-initialization-and-direct-initializati – jpo38 Oct 02 '14 at 09:34
3

I think the different behaviours you're experiencing are due to compiler optimization.

In any case,

Foo baz = fooFunc();

needs a copy constructor to work, because it will construct a new object from fooFunc's returned one.

For instance

Foo(const Foo& foo) : _BarList( NULL )
{
    ::operator=( foo );
}

This will guarantee fooFunc() return object is copied to the locally declared Foo baz object.

Note: Actually, calling operator= from copy constructor is a bad practice (but I mentioned it here to give you a quick answer and it will work in your case). Check this post: Copy constructor and = operator overload in C++: is a common function possible?

Community
  • 1
  • 1
jpo38
  • 20,821
  • 10
  • 70
  • 151
  • Sweet. Makes sense. Why the _BarList bit? – Beakie Oct 02 '14 at 08:30
  • You need to set _BarList to NULL, else, ::operator= will crash when doing `delete[] _BarList;`. – jpo38 Oct 02 '14 at 08:32
  • So, if I have no items in _BarList (ie, I haven't called new[] on it), setting it to NULL (in my case, 0) will not cause it to crash when I then call delete[] on it? – Beakie Oct 02 '14 at 08:34
  • 1
    Yes, calling `delete` on `0` is safe – Johannes S. Oct 02 '14 at 08:39
  • Your `operator=` will first delete `this->_BarList` and then copy foo's `_BarList` content to `this->_BarList`. But `delete [] _BarList` will crash if `_BarList` was not initialized. `delete [] NULL` does not crash. – jpo38 Oct 02 '14 at 08:40