operator overloading = modifies original object

Question

struct List {
    int size;
    int* items;
    List& operator=(const List& l);
};

List& List::operator=(const List& l)
{
    delete[] items;
    size = l.size;
    items = new int[20];

    for (int i = 0; i < size; i++)
        items[i] = l.items[i];

    return *this;
}

ostream& operator<<(ostream& out, const List& l)
{
    out << "Size: " << l.size << "\t {";
    for (int i = 0; i < l.size; i++)
        out << l.items[i] << " ";

    return (out << "}");
}

int main()
{
    int size = 6;
    List l1 = { size, new int[size]{ 0, 1, 2, 3, 4, 5 } };
    List l2 = l1;

    l2.items[1] = 50;

    cout << l1 << endl;
    cout << l2 << endl;

    return 0;
}

I'm confused since when I assign l2 = l1 by using an overloaded operator, why does the contents of l1 change, when altering l2 later on? Especially since l1 is passed as a const. They somehow point to the same object in memory instead of being a copy.

Please include the output and expected output in the question — 463035818_is_not_an_ai, Dec 19 '18 at 17:14
@PaulMcKenzie But when first declaring l2, and then using the same operator on a separate line it works. Why? `List l2 = { size, new int[size] }; l2 = l1;` — snzm, Dec 19 '18 at 17:17
@snzm -- I suggest you follow the answers and links to the rule of 3. Your assumptions about when, where (and even how) to properly copy an object are seriously flawed. — PaulMcKenzie, Dec 19 '18 at 17:19

NathanOliver · Answer 1 · 2018-12-19T17:28:59.237

List l2 = l1; does not call the copy assignment operator (operator=(const List& l)). Since the "assignment" happens during a variable declaration you are instead initializing l2 via copy initialization which is calling the compiler generated default copy constructor. Since it does a shallow copy both objects will point to the same data.

If you are going to be writing your own classes that manage memory/resources you need to at least provide your own copy constructor, copy assignment operator and destrcutor. This is known as the rule of three. If you want to include move semantics then you need to provide a move constructor and move assignment operator, which is known as the rule of 5. There is also the rule of zero, where use use types that already "do the right thing" (like using a std::vector) allowing the compiler generated defaults to work for you and you can read about all it at The rule of three/five/zero

@Bathsheba I've dared. I even mentioned the rule of zero for completeness sake. — NathanOliver, Dec 19 '18 at 17:20
OP, also make sure to read about the [*copy&swap idiom*](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). It's makes writing copy/move assignment operator a no-brainer in 99% cases, as long as you already have a copy/move constructor. — HolyBlackCat, Dec 19 '18 at 17:26

Lightness Races in Orbit · Accepted Answer · 2018-12-19T17:18:42.393

List l2 = l1;

Despite the =, because this is a declaration you're performing copy construction (formally "copy-initialisation"), which has nothing to do with the assignment operator.

You didn't define a copy constructor (which should look much like your copy assignment operator), so the pointers are indeed shared.

The results would have been what you expected, had you written:

List l2{};
l2 = l1;

By the way, I'd give size and items defaults (0 and nullptr respectively), if I were you. Otherwise, when you forget that {}, the members have unspecified values and all hell breaks loose. This could be done with a nice default constructor, or by forgetting this whole enterprise and using a std::vector instead ;)

Bathsheba · Answer 3 · 2018-12-19T17:27:36.703

3

List l2 = l1; calls the compiler-generated copy constructor, which does not deep copy the pointer member.

If you were to use a std::vector for items then you could leave the constructors and assignment operator to the compiler. (You don't have any delete[] calls in your code which means that your class will leak memory like a colander leaks water.)

edited Dec 19 '18 at 17:27

answered Dec 19 '18 at 17:14

Bathsheba

231,907
34
361
483

operator overloading = modifies original object

3 Answers3