C++ overloading the = operator

Question

When overloading the = operator, should one make the contents of one object equal to the contents of the other object OR do you make the pointer of the object point to the same object?

Reading back on the question it seems that the contents should be copied and not the pointers. But I just can't figure it out, So I would be grateful if someone would explain what I should do, I know how to do both, I'm just not sure which one to choose.

class IntObject
{
private:
    int *pi_One;

public:

    IntObject(void);

    IntObject::IntObject(int const &i_one);

    ~IntObject(void);

    IntObject & operator=(const IntObject&);

};

IntObject::IntObject()
{
    pi_One = new int(0); 
}

IntObject::IntObject(int const &i_one)
{
    pi_One = new int(i_one); 
}

IntObject::~IntObject(void)
{
    delete pi_One;
}

IntObject & IntObject::operator=(const IntObject& c) {
//This copies the pointer to the ints
    this->pi_One = c.pi_One;

    return *this;  
}

Pointers already implicitly implement the = operator. So it's not clear how you would even override that. I suggest you give an example of two versions you're contemplating. — pmdj, Aug 01 '12 at 16:28
If you're struggling to figure this out you should probably read the excellent `Effective C++` and follow up books by Scott Meyers. — Jonathan Wakely, Aug 01 '12 at 18:34

score 3 · Accepted Answer · answered Aug 01 '12 at 16:32

3

It depends on what semantics you want to have in your type. If you want value semantics, then copy the contents (deep copy, as is the case in std::vector), if you want reference semantics (shallow copy, as in std::shared_ptr)

answered Aug 01 '12 at 16:32

David Rodríguez - dribeas

204,818
23
294
489

1

The semantics can be deduces from `IntObject::~IntObject`. Since it deletes `pi_One`, the class has value semantics, and needs a deep copy. A shallow copy would leak the old pointer and double-delete the new. – MSalters Aug 02 '12 at 07:52
@MSalters: You are right in that the current implementation leads to that conclusion, agreed. (The question of whether the current implementation models the intention or not is a different issue :) ) – David Rodríguez - dribeas Aug 02 '12 at 13:00

Paul Manta · Answer 2 · 2012-08-01T19:00:53.643

2

You should definitely copy the contents, not the pointers. Think about what you will do when one of the objects which both hold the same pointer is destroyed; you can't delete the pointer because the other object would be affected as well, but you can't not delete it either because you'd cause memory leaks. You'd have to use reference counting and everything would get a whole lot more complicated.

edited Aug 01 '12 at 19:00

answered Aug 01 '12 at 16:45

Paul Manta

30,618
31
128
208

score 0 · Answer 3 · answered Aug 01 '12 at 16:29

The contents should be copied (in fact, changing the pointer of the object shouldn't actually be possible - I can't imagine how you would do that - and even if it is somehow, you're not supposed to do it). You also have to take care of the differences between deep and shallow copies, especially if your class contains pointers (or containers with pointers in them).

Now that I think about it, I'm not even sure which pointer you could possibly want to reassign. Unless you are already working with a pointer - those already have an '=' operator that shouldn't be overloaded though.

score 0 · Answer 4 · answered Aug 01 '12 at 16:29

0

The principle of least astonishment would say to copy the content. When using operator= on any other object, you wouldn't expect it to copy pointers.

answered Aug 01 '12 at 16:29

Littlegator

385
4
18

score 0 · Answer 5 · answered Aug 01 '12 at 16:45

0

If you keep your destructor as it is, then you should change assignment overload. It would be also wise that nobody is attempting assigning IntObject to itself:

IntObject & IntObject::operator=(const IntObject& c) {
    if (this != &c)
    {
        //This copies the pointer to the ints
        *this->pi_One = *c.pi_One;
    }
    return *this;  
}

Otherwise, there will be attempt to free freed memory in IntObject's destructor

answered Aug 01 '12 at 16:45

Roman Saveljev

2,544
1
21
20

The comment is wrong: your code assigns the `int`, not the `int*`. Therefore, it is perfectly fine to omit the `if` statement as well. `*this->pi_One = *this->pi_One` is a valid assignment. (Removing the `if` generates better code. CPU's don't like branches). – MSalters Aug 02 '12 at 07:55
I agree in this very case not much (if at all) time would be saved. I value your deep knowledge of CPU internals and how you imply pipeline is flushed upon branching, but "the least astonishment principle" suggests my comment is not wrong as such ;) – Roman Saveljev Aug 02 '12 at 08:01
The check in general is a red flag. See http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom. In this case copy-and-swap isn't necessary, but for the same reasons the check is equally redundant. – MSalters Aug 02 '12 at 08:08

C++ overloading the = operator

5 Answers5