Problem with operator overloading

Question

I have a scene class with member variables Image **images, int * xcoords, int * ycoords. Now, I'm trying to overload my = operator.

I get the following memory errors (using valgrind)

Conditional jump or move depends on uninitialised value(s)
==6439==    at 0x406FCA: Scene::drawscene() const (scene.cpp:160)
==6439==    by 0x4084C1: main (testscene.cpp:50)

And the line in question from above (scene.cpp:160) is

if (images[i]!=NULL)

So theyre saying that images was not initialized.

And so anywhere else drawscene() was called did not cause any problems, but I think because the = operator was used, it caused a problem.

Can anyone see any problems in my code that could cause this error?

Remember what I said about the [copy-and-swap idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom)? Namely, to use it? — GManNickG, Feb 05 '11 at 02:01
Yes but this is the way we are supposed to implement it for this class. I think I got it almost right, but I cant see where the bug is. — iRobot, Feb 05 '11 at 02:04
I voted your question down largely because while it isn't a bad question, it is very specific and deserves at most to be a 3, and probably a 2 or maybe even a 1. — Omnifarious, Feb 05 '11 at 02:53

score 3 · Accepted Answer · edited May 23 '17 at 11:55

You need this:

        if(source.images[i]!=NULL) {
            images[i]=new Image;
            *images[i]=*source.images[i];
            xcoords[i]=source.xcoords[i];
            ycoords[i]=source.ycoords[i];
        } else {
            images[i] = NULL;
        }

and that will solve your immediate problem. But really, you're going about this the wrong way. As @Gman said, you should be using the copy-and-swap idiom.

The reason to use the idiom is exception safety. If something throws an exception in the middle of that operator = (and there is plenty that can since it's doing so much) you will be leaving the object in an undefined state, and that's very bad. The copy and swap idiom lets you write an exception safe copy-constructor (which is still a little tricky) and then leverage it to build an assignment operator.

As an added bonus, you get a working copy constructor and a swap function. The copy constructor is very handy for stuffing things in STL containers and passing or returning things by value. And the swap function is is pretty darned useful for users of your class, especially ones who would like to implement their own exception safe copy constructors and assignment operators.

score 0 · Answer 2 · answered Feb 05 '11 at 02:16

0

You only initialize images[i] if source.images[i] is non-NULL. If source.images[i] is NULL, you leave random garbage in images[i]

answered Feb 05 '11 at 02:16

Chris Dodd

119,907
13
134
226

Problem with operator overloading

2 Answers2