0

I haven't been able to find a good answer to this question.

I'm working on a C++ program and I'm trying to implement a function named copy which takes a reference to another object as an argument. Then, it returns a deep copy of this object.

Some background on my project: the Scene class contains a dynamic array (called "Images") of pointers to either NULL or an instance of the Image class, which is not shown here - but works as it should (it inherits all of its methods from a third party library, EasyBMP)

The reason I'm doing this is to avoid duplicating code in two places, but it's very possible that I'm taking the wrong approach.

I call this function in my assignment operator:

Scene const & Scene::operator=(Scene const & source)
{
    if (this != &source) {
        clear();
        copy(source);
    }
    return *this;
}

And my copy constructor:

Scene::Scene(Scene const & source)
{
    copy(source);
}

Finally, my copy() method looks like this:

Scene const & Scene::copy(Scene const & source)
{
    Scene res(source.Max);
    for (int i=0; i<res.Max; i++)
    {
        delete res.Images[i];
        if (source.Images[i] != NULL) 
            res.Images[i] = new Image(*(source.Images[i]));
        else
            res.Images[i] = NULL;
    }   

    return res;
}

Currently, it does NOT work. One problem I can see is that I'm trying to return a variable that goes out of scope as soon as the copy function ends. I tried returning a reference before, but the compiler threw errors and this wouldn't help with the scope issue anyways.

But I'm not even sure that my logic is right, i.e. can you even do something like this in a constructor? Or should I just explicitly write out the code in the copy constructor and assignment operator (without implementing the helper method copy)?

I'm very new to C++ and pointers, so any guidance would be much appreciated.

mwoz
  • 5
  • 1
  • 2
  • [Obligatory link to a guide of introductory C++ books for C++ newcomers](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). Seriously, if you're new to C++ as you've said, pick up a good C++ book and read through it. – In silico Sep 08 '11 at 04:43
  • Actually, I'm reading several at the moment -- but thanks for the link I'll be sure to take a look at those (plentiful!) resources! – mwoz Sep 08 '11 at 05:03
  • Ah, that's good. Don't hesitate to ask on Stack Overflow if you have *specific* problems during your studies. – In silico Sep 08 '11 at 05:05

2 Answers2

3

There's a way easier and more idiomatic way to do what you want: the copy-and-swap idiom.

// N.B. Not tested, but shows the basic structure of the copy-and-swap idiom.
class Scene
{
public:
    Scene(int)
    {
        // Initialize a pointer array of Images
    }

    ~Scene()
    {
        // Get rid of our pointer array of Images
    }

    // Copy constructor
    // N.B. Not exception safe!
    Scene(const Scene& rhs) : imgPtrArray(new Image*[rhs.max])
    {
        // Perform deep copy of rhs
        for (int i=0; i < rhs.max; ++i)
        {
            if (rhs.imgPtrArray[i] != 0)    
                imgPtrArray[i] = new Image(*(rhs.imgPtrArray[i]));
            else   
                imgPtrArray[i] = 0;   
        }      
    }

    // Copy assignment constructor
    // When this is called, a temporary copy of Scene called rhs will be made.
    // The above copy constructor will then be called. We then swap the
    // members so that this Scene will have the copy and the temporary
    // will destroy what we had.
    Scene& operator=(Scene rhs)
    {
        swap(rhs);
        return *this;
    }

    void swap(Scene& rhs)
    {
        // You can also use std::swap() on imgPtrArray
        // and max.
        Images** temp = imgPtrArray;
        imgPtrArray = rhs.imgPtrArray;
        rhs.imgPtrArray = temp;
        int maxTemp = max;
        max = rhs.max;
        rhs.max = maxTemp;
    }

private:
    Images** imgPtrArray;
    int max;
};

That being said, I highly recommend that you pick up a good introductory C++ book, which will cover the basics of implementing the copy constructor and copy assignment operators correctly.

Community
  • 1
  • 1
In silico
  • 51,091
  • 10
  • 150
  • 143
0
Scene const & Scene::operator=(Scene const & source);

overloaded assignment operator copies the content of this to the argument received source. For copy there is no need to return any thing or to create a local object. Just make a member wise copy from this to source.

 void Scene::copy(Scene const & source){
     // Member wise copy from this to source
 }

Rule of three should be helpful to better understand more about these.

Community
  • 1
  • 1
Mahesh
  • 34,573
  • 20
  • 89
  • 115