Why does overloaded operator= destroy my operands?

J

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
 
A

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

Artie Gold

Jerry said:
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
 
J

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
 
A

Andre Kostur

(e-mail address removed) (Alf P. Steinbach) wrote in @news.individual.net:
* Jerry Krinock:

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.
 
A

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;
}
 
J

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
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,997
Messages
2,570,241
Members
46,831
Latest member
RusselWill

Latest Threads

Top