Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C++ > Destructors And Operator Overloading

Reply
Thread Tools

Destructors And Operator Overloading

 
 
רמי
Guest
Posts: n/a
 
      04-10-2008
Hey,

I've created a small class that overrides the "+" operator, and
defines a destructor:

private:
T *data;
int size;
public:
myClass(int s);
myClass<T> operator+(const myClass<T>& vec);
~myClass();

The implementation of the operator is:
myClass<T> myClass<T>:perator+(const myClass<T>& cls)
{
int i;
myClass<T> res(cls.size);

for (i=0;i<cls.size;i++)
{
res.data[i] = data[i];
}

return res;
}

Don't judge the code as its just a pseudo of the real one that.

The destructor is:

myClass<T>::~myClass()
{
delete [] data;
}

The problem is, that after the last line of the operator+ - "return
res"
I don't know why, but the destructor is called for "res",
Which means that the object returned, is already cleaned!!!

What am I missing\doing wrong?

Thanks ahead

--sternr
 
Reply With Quote
 
 
 
 
Barry
Guest
Posts: n/a
 
      04-10-2008
רמי wrote:
> Hey,
>
> I've created a small class that overrides the "+" operator, and
> defines a destructor:
>
> private:
> T *data;
> int size;
> public:
> myClass(int s);
> myClass<T> operator+(const myClass<T>& vec);
> ~myClass();
>
> The implementation of the operator is:
> myClass<T> myClass<T>:perator+(const myClass<T>& cls)
> {
> int i;
> myClass<T> res(cls.size);
>
> for (i=0;i<cls.size;i++)
> {
> res.data[i] = data[i];
> }
>
> return res;
> }
>
> Don't judge the code as its just a pseudo of the real one that.
>
> The destructor is:
>
> myClass<T>::~myClass()
> {
> delete [] data;
> }
>
> The problem is, that after the last line of the operator+ - "return
> res"
> I don't know why, but the destructor is called for "res",
> Which means that the object returned, is already cleaned!!!
>


a temporary instance of myClass<T> is contructed via "Copy Constructor"
to copy "res" you just returned. At the end of the function block, "res"
is destructed.

But RVO/NRVO ([Named] Return Value optimization) technique is used to
avoid the copy of for the temp variable.

you can google it.
 
Reply With Quote
 
 
 
 
dertopper@web.de
Guest
Posts: n/a
 
      04-10-2008
On 10 Apr., 10:41, רמי <(E-Mail Removed)> wrote:
> Hey,
>
> I've created a small class that overrides the "+" operator, and
> defines a destructor:
>
> private:
> T *data;
> int size;
> public:
> myClass(int s);
> myClass<T> operator+(const myClass<T>& vec);
> ~myClass();
>
> The implementation of the operator is:
> myClass<T> myClass<T>:perator+(const myClass<T>& cls)
> {
> * * * * * * * * int i;
> * * * * myClass<T> res(cls.size);
>
> * * * * for (i=0;i<cls.size;i++)
> * * * * {
> * * * * * * * res.data[i] = data[i];
> * * * * }
>
> * * * * return res;
>
> }
>
> Don't judge the code as its just a pseudo of the real one that.
>
> The destructor is:
>
> myClass<T>::~myClass()
> {
> * * * * * * * delete [] data;
>
> }
>
> The problem is, that after the last line of the operator+ - "return
> res"
> I don't know why, but the destructor is called for "res",
> Which means that the object returned, is already cleaned!!!


That is only to be expected: You declare a temporary object called
"res" which will be filled with the concatenated sequence of elements.
When the operator+ returns, this variable will be copied to the
variable that holds the return value in the calling function. After
this copy, "res" will be destroyed (unless your compiler performs some
kind of return value optimization). Thus your class needs a copy
constructor, so that the copy that is seen by the caller of operator+
receives a proper copy of "res".

BTW, your operator+ should have the following signature:
_const_ myClass<T> operator+(const myClass<T>& vec) _const_;
The first additional const ensures that the following code will be
prohibited:
myClass<int> a, b, c;
(a + b) = c;
The second const states that your operator+ doesn't change anything
for the object it is invoked on. This allows you to add const objects:
const myClass<int> a, b;
a + b;

Regards,
Stuart
 
Reply With Quote
 
Barry
Guest
Posts: n/a
 
      04-10-2008
http://www.velocityreviews.com/forums/(E-Mail Removed) wrote:
> On 10 Apr., 10:41, רמי <(E-Mail Removed)> wrote:
> BTW, your operator+ should have the following signature:
> _const_ myClass<T> operator+(const myClass<T>& vec) _const_;
> The first additional const ensures that the following code will be
> prohibited:
> myClass<int> a, b, c;
> (a + b) = c;
> The second const states that your operator+ doesn't change anything
> for the object it is invoked on. This allows you to add const objects:
> const myClass<int> a, b;
> a + b;


I think the second case depends.

maybe we also want
a + b + c;
 
Reply With Quote
 
Michael.Boehnisch@gmail.com
Guest
Posts: n/a
 
      04-10-2008
On 10 Apr., 10:41, רמי <(E-Mail Removed)> wrote:
[..]
> The implementation of the operator is:
> myClass<T> myClass<T>:perator+(const myClass<T>& cls)
> {
> int i;
> myClass<T> res(cls.size);
>
> for (i=0;i<cls.size;i++)
> {
> res.data[i] = data[i];
> }
>
> return res;
>
> }
>

[..]
> The problem is, that after the last line of the operator+ - "return
> res"
> I don't know why, but the destructor is called for "res",
> Which means that the object returned, is already cleaned!!!
>
> What am I missing\doing wrong?


Your implementation creates a local variable res that is destroyed
when leaving the function body, hence the destructor call.
"return res" provides a copy of res to the caller.

However, in the member "data", a copy of the pointer is provided that
is used to store your array in local variable. When your destructor
frees the memory, the pointer in the returned object becomes invalid.
You have two options:
1. provide a copy constructor that does a "deep copy". I.e. allocates
a new buffer and copies the memory content pointed to.
2. avoid T*, use a std::vector<T> instead. This will do all memory
allocation issues transparently for you, no need for memory
deallocation in a destructor, no need to write a copy constructor
yourself.

The solution 2. is what I would prefer. Your adapted class may look
like this:

template <class T>
class myClass {
private:
std::vector<T> data;
/* int size; */ // obsolete, use data.size() if you have to.

public:
myClass( int s );
myClass<T> operator+( const myClass<T>& vec );
/* ~myClass(); */ // obsolete, default destructor handles data
member
};

If "data" is *the* central member of your class, to make handling more
convenient I'd also consider inheritance:

template <class T>
class myClass : private std::vector<T> {
...
};

The implementation of your + operator will become:

template <class T>
myClass<T>:perator + ( const myClass<T>& cls ) {
return *this = cls; // that's probably not what you intended?
}


best,

Michael.
 
Reply With Quote
 
רמי
Guest
Posts: n/a
 
      04-10-2008
On Apr 10, 12:12*pm, Barry <(E-Mail Removed)> wrote:
> רמי wrote:
> > Hey,

>
> > I've created a small class that overrides the "+" operator, and
> > defines a destructor:

>
> > private:
> > T *data;
> > int size;
> > public:
> > myClass(int s);
> > myClass<T> operator+(const myClass<T>& vec);
> > ~myClass();

>
> > The implementation of the operator is:
> > myClass<T> myClass<T>:perator+(const myClass<T>& cls)
> > {
> > * * * * * * * * int i;
> > * *myClass<T> res(cls.size);

>
> > * *for (i=0;i<cls.size;i++)
> > * *{
> > * * * * *res.data[i] = data[i];
> > * *}

>
> > * *return res;
> > }

>
> > Don't judge the code as its just a pseudo of the real one that.

>
> > The destructor is:

>
> > myClass<T>::~myClass()
> > {
> > * * * * * * * delete [] data;
> > }

>
> > The problem is, that after the last line of the operator+ - "return
> > res"
> > I don't know why, but the destructor is called for "res",
> > Which means that the object returned, is already cleaned!!!

>
> a temporary instance of myClass<T> is contructed via "Copy Constructor"
> to copy "res" you just returned. At the end of the function block, "res"
> is destructed.
>
> But RVO/NRVO ([Named] Return Value optimization) technique is used to
> avoid the copy of for the temp variable.
>
> you can google it.- Hide quoted text -
>
> - Show quoted text -


So, how am I suppose to pass an instance of myClass<T>?

--sternr
 
Reply With Quote
 
רמי
Guest
Posts: n/a
 
      04-10-2008
On Apr 10, 12:48*pm, (E-Mail Removed) wrote:
> On 10 Apr., 10:41, רמי <(E-Mail Removed)> wrote:
> [..]
>
>
>
>
>
> > The implementation of the operator is:
> > myClass<T> myClass<T>:perator+(const myClass<T>& cls)
> > {
> > * * * * * * * * int i;
> > * * * * myClass<T> res(cls.size);

>
> > * * * * for (i=0;i<cls.size;i++)
> > * * * * {
> > * * * * * * * res.data[i] = data[i];
> > * * * * }

>
> > * * * * return res;

>
> > }

>
> [..]
> > The problem is, that after the last line of the operator+ - "return
> > res"
> > I don't know why, but the destructor is called for "res",
> > Which means that the object returned, is already cleaned!!!

>
> > What am I missing\doing wrong?

>
> Your implementation creates a local variable res that is destroyed
> when leaving the function body, hence the destructor call.
> "return res" provides a copy of res to the caller.
>
> However, in the member "data", a copy of the pointer is provided that
> is used to store your array in local variable. When your destructor
> frees the memory, the pointer in the returned object becomes invalid.
> You have two options:
> 1. provide a copy constructor that does a "deep copy". I.e. allocates
> a new buffer and copies the memory content pointed to.
> 2. avoid T*, use a std::vector<T> instead. This will do all memory
> allocation issues transparently for you, no need for memory
> deallocation in a destructor, no need to write a copy constructor
> yourself.
>
> The solution 2. is what I would prefer. Your adapted class may look
> like this:
>
> template <class T>
> class myClass {
> private:
> * * std::vector<T> data;
> * * /* int size; */ // obsolete, use data.size() if you have to.
>
> public:
> * * myClass( int s );
> * * myClass<T> operator+( const myClass<T>& vec );
> * * /* ~myClass(); */ // obsolete, default destructor handles data
> member
>
> };
>
> If "data" is *the* central member of your class, to make handling more
> convenient I'd also consider inheritance:
>
> template <class T>
> class myClass : private std::vector<T> {
> * * ...
>
> };
>
> The implementation of your + operator will become:
>
> template <class T>
> myClass<T>:perator + ( const myClass<T>& cls ) {
> * * return *this = cls; // that's probably not what you intended?
>
> }
>
> best,
>
> * *Michael.- Hide quoted text -
>
> - Show quoted text -


Ok, so I understand that if I'll add a:
myClass(const myClass<T>& cls);

it will work.

But what if I overloaded the operator= as well (for other reasons),
Than if I perform this line:
cls3 = cls1 + cls2

What will happen is that 2 instances will be created:
One from "cls1 + cls2" - by the new copy constructor,
And another from the "cls3 = result" by the operator=

How can I minimize that?

Thanks all

--sternr
 
Reply With Quote
 
Michael.Boehnisch@gmail.com
Guest
Posts: n/a
 
      04-10-2008
On 10 Apr., 12:08, רמי <(E-Mail Removed)> wrote:
[..]
> What will happen is that 2 instances will be created:
> One from "cls1 + cls2" - by the new copy conad structor,
> And another from the "cls3 = result" by the operator=
>
> How can I minimize that?


Switch on optimization. Modern compilers will detect the situation and
generate code that avoids excessive copying of objects on return. I.e.
they optimize away the local variable completely and perform the
operation directly in the returned temporary.

Ancient g++ offered as proprietary extension (a later deprecated and
removed feature) an explicitly declared return object.

best,

Michael
 
Reply With Quote
 
Thomas J. Gritzan
Guest
Posts: n/a
 
      04-10-2008
רמי wrote:
> I've created a small class that overrides the "+" operator, and
> defines a destructor:
>
> private:
> T *data;
> int size;
> public:
> myClass(int s);
> myClass<T> operator+(const myClass<T>& vec);
> ~myClass();
>
> The implementation of the operator is:
> myClass<T> myClass<T>:perator+(const myClass<T>& cls)
> {
> int i;
> myClass<T> res(cls.size);
>
> for (i=0;i<cls.size;i++)
> {
> res.data[i] = data[i];
> }
>
> return res;
> }
>
> Don't judge the code as its just a pseudo of the real one that.
>
> The destructor is:
>
> myClass<T>::~myClass()
> {
> delete [] data;
> }
>
> The problem is, that after the last line of the operator+ - "return
> res"
> I don't know why, but the destructor is called for "res",
> Which means that the object returned, is already cleaned!!!
>
> What am I missing\doing wrong?


Since this is a classic violation of the "Rule of three", I wonder why
nobody else mentioned it:
http://en.wikipedia.org/wiki/Rule_of...programming%29

You most probably should define a destructor, a copy constructor and a copy
assignment operator, if you define one of them.

--
Thomas
http://www.netmeister.org/news/learn2quote.html
sigfault
 
Reply With Quote
 
רמי
Guest
Posts: n/a
 
      04-10-2008
On Apr 10, 1:30*pm, (E-Mail Removed) wrote:
> On 10 Apr., 12:08, רמי <(E-Mail Removed)> wrote:
> [..]
>
> > What will happen is that 2 instances will be created:
> > One from "cls1 + cls2" - by the new copy conad structor,
> > And another from the "cls3 = result" by the operator=

>
> > How can I minimize that?

>
> Switch on optimization. Modern compilers will detect the situation and
> generate code that avoids excessive copying of objects on return. I.e.
> they optimize away the local variable completely and perform the
> operation directly in the returned temporary.
>
> Ancient g++ offered as proprietary extension (a later deprecated and
> removed feature) an explicitly declared return object.
>
> best,
>
> * *Michael


Hey Michael,
Thanks for the fast reply
Are you sure there's no "coding" solution for this duality?

--sternr
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
overloading operator->*() and operator->() gob00st@googlemail.com C++ 2 02-21-2009 04:26 AM
overloading operator->*() and operator->() gob00st@googlemail.com C++ 11 02-20-2009 08:52 PM
Why is overloading operator. (member operator) forbidden? dascandy@gmail.com C++ 11 05-16-2007 07:54 PM
Help with delete operator and destructors (code attached) kbubbar@hotmail.com C++ 4 10-28-2006 10:25 PM
Operator overloading on "default" operator John Smith C++ 2 10-06-2004 10:22 AM



Advertisments