Why does overloaded operator= destroy my operands?

[Jerry Krinock]

I'm writing a class which has, as members, dynamically allocated
valarrays. I'd like to overload the "=" operator so that, when
operating on two objects of my class, the valarray values of the rhs
will be assigned to the valarray values of the lhs.

Well, in doing so I have discovered something I don't understand about
this operator; it destroys my operands upon return. I've written the
following demo, simplified to use ints instead of valarrays:

#include <iostream>
using namespace std ;

class Foo
{
private:
char me ;
int* num ;

public:
Foo(char me_, int num_) //constructor
{
me = me_ ;
num = new int ;
*num = num_;
}

int GetNum() const // getter
{
return *num ;
}

Foo operator=(const Foo rhs)
{
*num = rhs.GetNum() ;
return *this ;
}

void print()
{
cout << me << " = " << *num << endl ;
}

~Foo()
{
cout << "Destroying " << me << endl ;
delete num ;
}
};
int main ()
{
Foo a( 'a', 2);
Foo b( 'b', 3);

cout << "Before operating..." << endl ;
a.print();
b.print();

a=b;

cout << "After operating..." << endl ;
a.print();
b.print();

cout << "All done!!" << endl ;
return 0;

}

Here is the output:

Before operating...
a = 2
b = 3
Destroying a
Destroying b
After operating...
a = 13691
b = 3
All done!!
Destroying b
*** malloc[683]: error for object 0x300470: Double free
Destroying a
*** malloc[683]: Deallocation of a pointer not malloced: 0x300460; This
could be........

Why does my overloaded operator= destroy its operands when it returns?
It doesn't even seem to care that I've declared the rhs as const!

Jerry Krinock
San Jose, CA USA

[Alf P. Steinbach]

* Jerry Krinock:

Foo operator=(const Foo rhs)
{
*num = rhs.GetNum() ;
return *this ;
}

Are you sure you want to return a copy instead of a reference?

Btw., it would be a Good Idea to support copying.

In other words, (also) add a copy constructor.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

[Artie Gold]

Jerry Krinock wrote:

I'm writing a class which has, as members, dynamically allocated
valarrays. I'd like to overload the "=" operator so that, when
operating on two objects of my class, the valarray values of the rhs
will be assigned to the valarray values of the lhs.

Well, in doing so I have discovered something I don't understand about
this operator; it destroys my operands upon return. I've written the
following demo, simplified to use ints instead of valarrays:

#include <iostream>
using namespace std ;

class Foo
{
private:
char me ;
int* num ;

public:
Foo(char me_, int num_) //constructor
{
me = me_ ;
num = new int ;
*num = num_;
}

int GetNum() const // getter
{
return *num ;
}

Foo operator=(const Foo rhs) Foo operator=(const Foo& rhs) {
*num = rhs.GetNum() ;
return *this ;
}

void print()
{
cout << me << " = " << *num << endl ;
}

~Foo()
{
cout << "Destroying " << me << endl ;
delete num ;
}
};
int main ()
{
Foo a( 'a', 2);
Foo b( 'b', 3);

cout << "Before operating..." << endl ;
a.print();
b.print();

a=b;

cout << "After operating..." << endl ;
a.print();
b.print();

cout << "All done!!" << endl ;
return 0;

}

Here is the output:

Before operating...
a = 2
b = 3
Destroying a
Destroying b
After operating...
a = 13691
b = 3
All done!!
Destroying b
*** malloc[683]: error for object 0x300470: Double free
Destroying a
*** malloc[683]: Deallocation of a pointer not malloced: 0x300460; This
could be........

Why does my overloaded operator= destroy its operands when it returns?
It doesn't even seem to care that I've declared the rhs as const!

What's being destroyed is the local copy -- taking the memory allocated
to its pointer-to-int with it when its dtor runs.

HTH,
--ag

--
Artie Gold -- Austin, Texas
http://it-matters.blogspot.com (new post 12/5)
http://www.cafepress.com/goldsays

[Jerry Krinock]

Thanks, Artie and Alf. I think I get most of it now. Since my
operator calls by value, a copy of the rhs is created. Furthermore,
when an object is returned by a function, it returns a temporary object
which is automatically created. Thus, both operands are copied when my
operator= is invoked. Since I did not have a copy constructor, these
were shallow copies. When I simply added a copy constructor,
everything worked as desired.

But, Alf, I think I am still returning a copy. How would I fix it to
return a reference, as you suggested?

Revised code and output follows:

Jerry

class Foo
{
private:
int* num ;

public:
char me ;
Foo(char me_, int num_)
{
me = me_ ;
num = new int ;
*num = num_;
}

Foo(const Foo &rhs) // copy constructor
{
me = rhs.me ;
num = new int ;
*num = rhs.GetNum() ;
cout << "Done copy construction of " << me << endl ;
}

int GetNum() const
{
return *num ;
}

Foo operator=(const Foo rhs)
{
*num = rhs.GetNum() ;
return *this ;
}

void print()
{
cout << me << " = " << *num << endl;
}

~Foo()
{
cout << "Destroying " << me << endl ;
delete num ;
}
};
int main ()
{
Foo a( 'a', 2);
Foo b( 'b', 3);

cout << "Before operating..." << endl ;
a.print();
b.print();

a=b;

cout << "After operating..." << endl ;
a.print();
b.print();

cout << "All done!!" << endl ;
return 0;
}

*** OUTPUT ***

Before operating...
a = 2
b = 3
Done copy construction of b
Done copy construction of a
Destroying a
Destroying b
After operating...
a = 3
b = 3
All done!!
Destroying b
Destroying a

[Andre Kostur]

al***@start.no (Alf P. Steinbach) wrote in news:4215200b.3 9221062
@news.individua l.net:

* Jerry Krinock:

Foo operator=(const Foo rhs)
{
*num = rhs.GetNum() ;
return *this ;
}

Are you sure you want to return a copy instead of a reference?

Btw., it would be a Good Idea to support copying.

In other words, (also) add a copy constructor.

In addition, it's taking Foo in by value as well... that should probably
also be by reference.

[Alf P. Steinbach]

* Jerry Krinock:

How would I fix it to return a reference, as you suggested?

Foo operator=(const Foo rhs)
{
*num = rhs.GetNum() ;
return *this ;
}

Foo& operator=( Foo const& rhs )
{
*num = rhs.GetNum();
return *this;
}

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

[Jerry Krinock]

Ah, of course!!! Since an operator is in fact a function, I can use the
reference parameters syntax! And when I call by reference, I see that
the copy constructor is not called, so my program will run faster,
although I shall keep the copy constructor for other purposes.

Thank you all very much for teaching this old electrical engineer such
cool new tricks.

Jerry