Destructors And Operator Overloading

[רמי]

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 = data;
}

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

 

[Barry]

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 = data;
}

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.

 

[dertopper]

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 = data;
        }

        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

 

[Barry]

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;

 

[Michael.Boehnisch]

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 = data;
}

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.

 

[רמי]

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 = data;
   }

   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

 

[רמי]

[..]

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 = data;
        }

        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

 

[Michael.Boehnisch]

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

 

[Thomas J. Gritzan]

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 = data;
}

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_three_(C++_programming)

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

 

[רמי]

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