Does the assignment operator call copy constructor?

Question

#include<iostream>
using namespace std;
class test
{
    int *p;
public:
    test(){p=new int(0);}
    test(const test &src)
    {
        p=new int(*src.p);
    }
    ~test(){delete p;}
    void show(){cout<<*p<<" at "<<p<<endl;}
    void setx(int a)
    {
        *p=a;
    }
};
int main()
{
    test a,b;
    a.setx(10);
    a.show();
    b=a;
    b.show();
    test c=a;
    c.show();
}

Here inside main(), test c=a calls the copy constructor and allocates memory for the integer. No problem with that, both c.p and a.p point to different memory locations. But the line b=a causes b.p and a.p to point to the same memory location. I have created my own copy constructor, b.p and a.p should have pointed to different memory locations. What's going on here?

Edit: To be precise, my question is what's the difference between implicitly-defined copy constructor and implicitly-defined copy assignment operator?

Does this answer your question? [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — Karsten Koop, Mar 16 '20 at 10:55
Assignment `a=b` does _not_ invoke the the copy-ctor. You will need to overload `operator=` for that. — Lukas-T, Mar 16 '20 at 10:56
memory leaks in `p=new int(a);` you must delete OLD `p` if it has a valid memory — Landstalker, Mar 16 '20 at 10:56
@Jose Thanks for noticing that. I have edited my post. Let me know if everything is alright now. — , Mar 16 '20 at 10:59
@user148469 I didn't want to say that more news are going to solve the problem, just highlighting the lack of a RAII pattern — Jose, Mar 16 '20 at 11:01
without the overload of your operator `=` you risk having a double free on `a.p` and `b.p` because it is the same address — Landstalker, Mar 16 '20 at 11:27
@KarstenKoop What's the difference between implicitly-defined copy constructor and implicitly-defined copy assignment operator? — , Mar 16 '20 at 11:45

srilakshmikanthanp · Accepted Answer · 2020-03-17T11:35:43.663

4

b=a;

Here also bit by bit copy is done (a.p and b.p pointing to same location), it does not invoke copy constructor because constructor is called when b in defined (default constructor).so you have to overload = operator

test &operator =(const test &src)
{
    *this->p=*src.p;    //copy value not address 

     return *this;
}

Add this to your class test and you need to check the memory is allocated or not by new because new can fail to allocate requested memory.

But here the copy constructor is called

test c=a;

edited Mar 17 '20 at 11:35

answered Mar 16 '20 at 11:04

srilakshmikanthanp

2,231
1
8
25

You do not want to pass `src` by value. `this->p=src.p;` would make `b.p` point to the same memory as `a.p` and that will crash when both want to delete it in the destructor, it also creates a memory leak for `a.p` as you are overwriting the pointer. Do you mean `*this->p=*src.p;`? – mch Mar 16 '20 at 11:10
@mch sorry for the mistake i did not note it , i would corrected it.Thanks – srilakshmikanthanp Mar 16 '20 at 11:27

Jose · Answer 2 · 2020-03-16T13:57:14.730

If you really want to use an *int, I'd use smart pointers, to simplify the resource ownership:

class Test
{
public:
    Test(){p=std::shared_ptr<int>(new int(0));}
    Test(const Test &src){p=std::shared_ptr<int>(new int(*(src.p.get())));}
    ~Test()=default;
    Test& operator= (const Test& src) {p = std::make_shared<int>(*(src.p.get())); return *this;}
    void show(){cout<<*(p.get())<<endl;}
    void setx(int a) {*(p.get())=a;}
private:
    std::shared_ptr<int> p;
};

however, it does not make too much sense to use a pointer or smart pointers (obfuscated code) considering that the lifespan of the int is the same than the object itself (so you don't have to dynamically allocate it):

class Test
{
public:
    Test():p(0){}
    Test(const Test &src):p(src.p){}
    ~Test()=default;
    Test& operator= (const Test& src) {p = src.p; return *this;}
    void show(){cout<<p<<endl;}
    void setx(int a) {p=a;}
private:
    int p;
};

C. Dunn · Answer 3 · 2020-03-16T14:42:05.900

0

You need to define operator=. The copy constructor won't be called for an already-constructed object.

e.g.,

test&operator=(const test&t)
{
    if(p)delete p;
    p=new int(*t.p);
    return *this;
}

edited Mar 16 '20 at 14:42

answered Mar 16 '20 at 11:04

C. Dunn

101
1
6

2

This will create a memory leak for `this->p`. You can pass `t` as `const test &`. – mch Mar 16 '20 at 11:12

Landstalker · Answer 4 · 2020-03-16T12:07:44.567

0

Try this:

test& operator= (const test& anotherTest)
{
    if (this != &anotherTest) 
    { 
        if ( NULL != this->p )
            *(this->p) = *(anotherTest.p);
        else
            this->p = new int (*anotherTest.p)
    }   

    return *this;
}

edited Mar 16 '20 at 12:07

answered Mar 16 '20 at 11:10

Landstalker

1,368
8
9

1

This will create a memory leak for `this->p`. – mch Mar 16 '20 at 11:11
you will do `delete p` in new instance destructor – Landstalker Mar 16 '20 at 11:12
`operator=` is not for new instances. Both instances are fully constructed, so both have a valid pointer `p` and you overwrite the one from the left side. It has nothing to do with the destructor, the destructor cannot call delete on an overwritten pointer. – mch Mar 16 '20 at 11:13
@mch ok, I thought it was only for new instances, thanks for the remark. (Edited) – Landstalker Mar 16 '20 at 11:24
1

Firstly, you don't need `if (NULL != this->p)`: if `p` is null, `delete p` safely does nothing. Secondly, why `this->p`? Simply `p` is enough. Thirdly, it's `nullptr` these days, not `NULL`. In summary: `delete p; p = new` etc. – TonyK Mar 16 '20 at 11:56
@TonyK `this->p` for better interpretation if the code will be maintained by another person ... – Landstalker Mar 16 '20 at 11:59
Oh, and fourthly: `p` is always allocated, so really you just need `*p = *anotherTest.p;` for the whole function. You don't even need to check for `this != &anotherTest`. – TonyK Mar 16 '20 at 12:00

score 0 · Answer 5 · answered Mar 16 '20 at 13:04

0

I dont know why, but if I:

test a;
a.setx(10);
a.show();
test b = a;
b.show();
test c = a;
c.show();

instead of :

test a,b;
a.setx(10);
a.show();
b=a;
b.show();
test c=a;
c.show();

a.p and b.p point to different addresses

answered Mar 16 '20 at 13:04

platinoob_

151
1
11

If you don't know _why_ it works it's no answer, is it? – Lukas-T Mar 17 '20 at 06:45
@churill you are correct, and probably I need to learn more about "=", but, the person who made this question might be in hurry, and I do not think that declare b at the time of its initialisation it's that much of a difference, it might be good enough for the "questioner", and later the "questioner" might search the reason the code in my answer works of its own – platinoob_ Mar 17 '20 at 08:01

Does the assignment operator call copy constructor?

5 Answers5