CODEWITHSUNDEEP
c++
Ilan Tal
Ilan Tal

Reputation: 499

What is the proper way to call the destructor to free up memory in a C++ operator?

I'm having trouble knowing what I'm doing wrong.

Huge& Huge::operator *=(const Huge& h) {
    Huge retHuge = *this * h;
    string out1 = retHuge.toString(); // for debugging purposes
    return *this = retHuge;
//  return retHuge;
}

The

Huge retHuge = *this * h;

works fine as I verify in the string out1. If I return retHuge and print the result in the calling routine, I can see that the original this remains unchanged. If I do a *this = retHuge and return *this, I get a SEGSEGV fault.

What is the proper way to call the destructor to clean up memory and to return the result after the multiply?

I would like to thank all of you for the answers. Crazy Eddie used what I had and extended it, which is exactly what I wanted to do. I changed the source to reflect his suggestion and the SEGSEGV fault comes up.

I take this to mean that my syntax must be basically correct but that I have a bug hidden in the rest of my code. This is valuable information and I need to go back and very carefully look to see if I can find something. I don't want to be lazy and ask for help before I do my homework properly, so I'll have a good look first.

My experience is mainly with Java which has a nice garbage collector for unreferenced objects. I'll go through with the debugger and verify that my destructor is being called to free the memory.

That was a good tip to try to do the *= operator first and then build the = operator from it. Clearly I did things in the opposite direction. Still your answers seem to indicate that what I did should indeed work. Since it doesn't work, I'll go back and see if I can find anything. If, after I have done my homework, I still can't find anything, I'll continue to ask. In the meantime, thanks for all the help.

Upvotes: 1

Views: 208

Answers (4)

sbi
sbi

Reputation: 224119

The canonical form of overloading arithmetic operators applied to your specific case: 

Huge& Huge::operator *=(const Huge& h) // class member function
{
  // somehow multiply h's value into this' value
  return *this;
}


Huge operator*(Huge lhs, const Huge& rhs) // free function; note lhs being copied
{
  lhs *= rhs;  // *= already implemented above
  return lhs;
}

There is no dynamic memory involved here at all, hence you do not need to delete any.

The compound assignment form should be a member according to the operator overloading rules of thumb because it changes it's left-hand operand (and is likely to need access to the class' private parts). The non-assigning version should be a free member function so that implicit conversions can be applied equally to both operands.

Upvotes: 8

Talya
Talya

Reputation: 19347

Do not delete this; afterwards, *this is writing into unallocated memory.

Instead, just say *this = retHuge (without delete this), then return *this.

Handle deallocation in operator = instead.

Here's a complete-ish example, with an imaginary complicated payload of an int *:

#include <iostream>

class A {
public:
    A(int _val): val(new int(_val))
    { }

    ~A() {
        delete this->val;
    }

    A &operator =(const A &r) {
        // Pretend this->val is complex and we need to reallocate the
        // entire thing.
        delete this->val;

        this->val = new int(*r.val);

        return *this;
    }

    A operator *(const A &r) const {
        return A(*this->val * *r.val);
    }

    A &operator *=(const A &r) {
        return *this = *this * r;
    }

    int *val;
};

int main() {
    A x(5), y(10);
    x *= y;

    std::cout << "x: " << *x.val << ", y: " << *y.val << std::endl;
    return 0;
}

Upvotes: 1

Edward Strange
Edward Strange

Reputation: 40877

Since you seem to already have the * operator, and I'll assume at this point that assignment works, this would be the best implementation:

Huge& Huge::operator *=(const Huge& h) {
    Huge retHuge = *this * h;

    return *this = retHuge;
}

Upvotes: 1

Yochai Timmer
Yochai Timmer

Reputation: 49261

operator= means you assign the right hand site into this.

So, you should change the contents of this, and then in the end return *this;

Upvotes: 0

Related Questions