c++ – Copy constructor not being called – Education Career Blog

I have a class which allocates memory on the heap and then the destructor frees it. My copy constructor is never being called for some reason and I do not understand why. Here is my implementation:

 AguiBitmap::AguiBitmap( const AguiBitmap &bmp )
    {

        this->nativeBitmapPtr = al_clone_bitmap(bmp.nativeBitmapPtr);
    }

    AguiBitmap::AguiBitmap( char *filename )
    {

        if(!filename)
        {
            nativeBitmapPtr = 0;
            return;
        }

        nativeBitmapPtr = al_load_bitmap(filename);

        if(nativeBitmapPtr)
        {

            width = al_get_bitmap_width(nativeBitmapPtr);
            height = al_get_bitmap_height(nativeBitmapPtr);
        }
        else
        {
            width = 0;
            height = 0;
        }
    }




    ALLEGRO_BITMAP* AguiBitmap::getBitmap() const
    {
        return nativeBitmapPtr;
    }

However,
When I do something like:

AguiBitmap bitmap;
bitmap = AguiBitmap("somepath");

The copy constructor code is never called (set a breakpoint). And therefore, my issue of having an invalid pointer in the reconstructed object from the temporary object becomes invalid when the temporary one is destroyed.

What do I do to get my copy constructor to be called?

Thanks

,

That bit of code wont invoke the copy constructor – it invokes the assignment operator (or the copy-assignment operator):

// a helper `swap` function
void AguiBitmap::swap(AguiBitmap& a, AguiBitmap& b)
{
    using std::swap;  // enable the following calls to come from `std::swap`
                      // if there's no better match

    swap(a.nativeBitmapPtr, b.nativeBitmapPtr);
    swap(a.width, b.width);
    swap(a.height,b.height);
}

AguiBitmap::AguiBitmap& operator=( const AguiBitmap &rhs )
{
    // use copy-swap idiom to perform assignment
    AguiBitmap tmp(rhs);

    swap( *this, tmp);
    return *this;
}

Also note that your copy constructor is incomplete, since the height and width members aren’t being copied:

width = bmp.width;
height = bmp.height;

,

AguiBitmap("somepath");

will invoke:

AguiBitmap::AguiBitmap( char *filename )

and the assignment will invoke the assignment operator

to invoke your copy constructor, do this:

AguiBitmap bitmap;
AguiBitmap anotherBitmap(bitmap)

,

Your code invokes the assignment operator, not the copy constructor. The copy constructor could be invoked for this code

AguiBitmap bitmap = AguiBitmap("somepath");

but the compiler might just as well optimize this and execute the equivalent of this instead:

AguiBitmap bitmap("somepath");

However, per the Rule of Three, when you have a copy constructor, you are very likely to need an assignment operator (and a destructor), too, anyway. So you should implement one.
See this answer for an idiomatic way to implement assignment on top of copy-construction and destruction.

,

Sorry, thats an assignment, not a copy

,

You can implement operator=. To avoid code duplication, use your copy constructor to implement it (since they should do pretty much the same thing). This will result in the copy constructor being called, if indirectly.

This question covers how to implement this stuff safely and efficiently.

,

You need something like this:

AguiBitmap &AguiBitmap::operator=(const AguiBitmap &guiBitmap)
{   
    if(&guiBitmap == NULL)
    {
      //throw Exception
    }

    if(&guiBitmap != this)
    {
      //do the copying here
       this->nativeBitmapPtr = al_clone_bitmap(guiBitmap.nativeBitmapPtr);
      //...
    }

    return *this;
}

Leave a Comment