CODEWITHSUNDEEP
c++classinheritance
Yitzhak Goldstine
Yitzhak Goldstine

Reputation: 13

object goes out of scope before = operation

I have thous objects:

Polygon p1, p2;

And I have an inheriting class of Polygon called Triangle, and I try to do:

p1 = Triangle(temp1, temp2, temp3); // temp 1,2,3 are lengths of sides

But for some reason the destructor for Triangle is called at the end of construction.

Rectangle::~Rectangle(void)
{
    Polygon::~Polygon();
}

Polygon::~Polygon(void)
{
    if (sides != NULL)
    {
        delete [] sides;
        sides = NULL;
    }
}

Then it runs the destructor for Polygon a second time.

so after the code ends this is what the debugger says about p1( n is number of sides):

p1  {n=3 sides=0x0062c070 {-17891602} } Polygon

Questions:

EDIT: as requested:

/*returns true if for the polygons A and B:
(a) for each side on polygon A there is an equal side on polygon B
(b) the number of sides in polygons A and B are equal
(c) sum of sides of A equals the sum of sides of B */
bool Polygon::operator==(const Polygon& p) const
{
    if (n != p.n)
        return false;
    if(circumference() != p.circumference())
        return false;
    for (int i=0; i < n; i++)
        if (!doesSideHasEqual(sides[i], p, n))
            return false;
    return true;
}

Also, thanks for the explanation why it ran ~Polygon, will take into account.

Upvotes: 0

Views: 134

Answers (3)

dascandy
dascandy

Reputation: 7292

As you're still learning and somebody else answered your question, here's some pointers on C++ programming.

Polygon p1, p2;

That's two objects of class Polygon.

p1 = Triangle(temp1, temp2, temp3); // temp 1,2,3 are lengths of sides

This constructs an object of class Triangle, and calls an assignment-operator on p1 to construct it from a Triangle. The Triangle is up-cast to a Polygon transparently and copy-constructed to p1, after which the Triangle temporary object is destroyed.

Rectangle::~Rectangle(void)
{
    Polygon::~Polygon();
}

This has one very major error and a subtle "old-style person" thing.

There is never a need to mention (void) in code; this was only sensible in C up to about 14 years ago. We're doing C++, and we're doing so in 2013, so don't use (void) but just use (). This already explicitly says no arguments are allowed.

The major problem though is that you're explicitly destructing the parent. The parent(s) are always implicitly destructed. This way they are first explicitly destructed, and then again implicitly destructed. Never call your destructor from your destructor.

Polygon::~Polygon(void)
{
    if (sides != NULL)
    {
        delete [] sides;
        sides = NULL;
    }
}

This reeks of "defensive programming". The code can be called multiple time and on half-constructed objects and it will not crash. On the other hand, you won't expose errors in other parts of your software either, so those will persist. A double-delete typically points to shared ownership and by extension a dangling pointer. This will mask that error perfectly.

Either decide that the sides member does have a value, or it does not. If it has a value, always delete it. For safety add an assert saying sides != nullptr or sides != NULL to check it beforehand, so that the code will crash with core file / stack trace if it doesn't.

If it may not have a value, still always delete it. Delete is safe for null pointer values, making for much cleaner code. This removes your check entirely. Setting it to NULL is allowed and will aid in finding use of a dangling pointer to this Polygon object.

Upvotes: 0

Adrian Shum
Adrian Shum

Reputation: 40036

Polygon p1;

p1 = Triangle(temp1, temp2, temp3); // temp 1,2,3 are lengths of sides

is in fact giving you result that is out of your expectation.

What you want to do is something like 

Polygon& p1;
Triangle t(temp1, temp2, temp3);

p1 = t;

or 

Polygon* p1 = new Triangle(temp1, temp2, temp3);

The problem in 

p1 = Triangle(temp1, temp2, temp3);

is not only the Triangle object is temporary and destroyed right after the statement, but, more seriously, the copy from the Triangle object to p1 which is the Polygon, is only copying as is a Polygon. Therefore, the "copied" p1 is not containing any information of Triangle.

Another problem is the way you write the destructor. Destructor in C++ is automatically chained (I think you are coming from Java background which we need to explicitly invoke super class' finalizer)

Rectangle::~Rectangle(void)
{
    Polygon::~Polygon();  // can be removed
}

Therefore you shouldn't call Polygon's destructor.

Another thing about destructor is, if your class is supposed to be inherited (your Polygon is a good example), you should declare your destructor to be virtual. For the reason behind, search for "virtual destructor".

Upvotes: 0

jcoder
jcoder

Reputation: 30035

This line:

p1 = Triangle(temp1, temp2, temp3);

Constructs a triangle object, then makes a copy of that object in p1, and the destroys the original object. It's the destructor for the original object you are seeing. (And it probably doesn't do what you want anyway because it will slice the object and only store the part of it that the base class has.)

Also you should not call the destructors for base classes in your destructors, they are called automatically.

And as a matter of style 

if (sides != NULL)
{
    delete [] sides;
    sides = NULL;
}

There is no need to test sides before deleting it as delete on a null pointer does nothing by design. And there is little point setting sides to NULL as after the destructor is run the object doesnt exist anyway.

Upvotes: 2

Related Questions