overload operator + c++ eclipse

Question

I'm trying to overload operator +. important to say, operator + gets exactly the datat I want it to get, but once i send it to operator=, all the pointers in it are deleted for some reason.

Movie& Movie:: operator+ (const Movie& other) {

Movie toReturn,toCheck;
Worker* toAdd;
std::list<Worker*>::iterator checkSecond;
length > other.getLength() ?  toReturn=*this, toCheck=other : toReturn =  
other , toCheck = *this;

//I WANT TO TAKE THE LONGER MOVIE, AND ADD THE WORKERS FROM THE SHORTER 
  MOVIE TO THAT MOVIE

if(toCheck.getNumOfWorkers() > 0 ) //LOOK FOR WORKERS IN SHORT MOVIE

{
for (checkSecond=toCheck.getWorkersInMovie().begin(); checkSecond != 
toCheck.getWorkersInMovie().end(); ++checkSecond)
           {
                toAdd= (*checkSecond);
                toReturn.addWorker(toAdd);
                toAdd = NULL;


            } //END FOR
    }
}
}

*this=toReturn;
cout << "this is what my new object has after +" <<endl; 
this->printMovie(); //PRINTS EXACTLY WHAT I EXPECT, WITH NEW WORKERS
return *this;

}

but when i send it to operator = it doesn't get copied as expected, say the number of workers is 0. MOVIE BOTH = MOVIEFIRST + MOVIETOADD

Movie& Movie:: operator= (const Movie& other) {

WHEN IT GETS HERE, MOVIE OTHER GETS DELETED..
return *this;

}

copy constructor works as expected in all scenarios:

Movie::Movie(const Movie& toCopy) {

  cout << "inside copy" << endl;
 *this=toCopy;
}

HELP...?

You would be better served if you implemented the "work" code in the copy constructor, and have `operator=` call the copy constructor via the `copy / swap` idiom. http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom Also, you should post *all* of the code in your assignment operator and copy constructor, not stuff like "When it gets here". You're asking a question, and holding back such important information doesn't help. — PaulMcKenzie, Dec 05 '15 at 10:36

score 2 · Answer 1 · answered Dec 05 '15 at 10:13

2

Operator + is invoked for expressions like A + B. Semantically, this expression shouldn't be changing the state of A but instead return a new object representing the sum.

Therefore the correct signature for an overloaded + operator should be the following:

Movie Movie::operator+ (const Movie& other) const { }

If you want to avoid creation of this temporary object, then you should not overload the + operator, but the += as follows:

Movie& Movie::operator+= (const Movie& other) { }

Since += has the assignment semantics, you are allowed to modify the object.

Following this rules will help you in solving the issues you are seeing with the code.

answered Dec 05 '15 at 10:13

simpel01

1,792
12
13

ok, but what if I want to overload a few movies together? I need A+B+C=D without changing A,B,C – aviadm71 Dec 05 '15 at 10:31
@aviadm71 The answer solves the issue of A+B+C without changing A,B, or C. – PaulMcKenzie Dec 05 '15 at 10:33
then overload the + operator with the signature I gave you above. I don't really understand what should A + B + C = D do? isn't that always the same as using D? – simpel01 Dec 05 '15 at 10:34
i want Movie D= A +B+C btw, in operator = should return Movie& ? – aviadm71 Dec 05 '15 at 10:44
@aviadm71 Yes it should return a reference to Movie, and the return value should be `*this`. Please look at the many *working* examples of implementing a correct assignment operator and copy constructor. That is a lot better than trial and error. – PaulMcKenzie Dec 05 '15 at 10:47
`Movie D = A + B + C;` will invoke the `Move` copy constructor, not the assignment operator. – simpel01 Dec 05 '15 at 10:50
Assignment operator is invoked on something like `D = A + B + C;` instead! – simpel01 Dec 05 '15 at 10:51

bitcode · Answer 2 · 2015-12-05T10:44:58.837

1

1. You have to overload the operator= properly if you want it to work. What you are currently doing in your operator= overload is returning its current state. You are not copying the values from the variable "other". This is what you should try to do:

    Movie& Movie:: operator= (const Movie& other) {

    //COPY EACH VALUE FROM 'OTHER' TO 'THIS'

    return *this;

    }

You are not assigning ANYTHING! You have to return the value after you have assigned them to this.

2. Your copy constructor should copy each element of the object it receives as argument - otherwise there is no need to have a copy constructor and you should use the one the compiler creates for you.

edited Dec 05 '15 at 10:44

answered Dec 05 '15 at 10:04

bitcode

343
4
15

It isn't just "important values" that should be copied. It should be *every* value copied, or else you have bogus copies. The one exception would be if the class is a smart pointer of some type or reference counted class. – PaulMcKenzie Dec 05 '15 at 10:42
@PaulMcKenzie just edited that out. But anyways, the way you implement operator overloads depends on your needs, IMO. Some times you don't want to copy every single value when you overload the assignment operator. – bitcode Dec 05 '15 at 10:48
Well, if your needs are endless nights debugging weird errors, then go for it. Seriously, the assignment operator is not under your control. You don't know when or where it will be called. Not providing actual copies introduces side-effects, and again, debugging errors because of these side-effects is no joke. – PaulMcKenzie Dec 05 '15 at 10:49

overload operator + c++ eclipse

2 Answers2