Basic question about destroying objects.

[Markus Pitha]

Hello,

I have a class Adjazenzmatrix with the following method and destructor:



private:
Adjazenzmatrix *wegmatrix;
Adjazenzmatrix *distanzmatrix;
..
..
..
Adjazenzmatrix::~Adjazenzmatrix() {
delete wegmatrix;
delete distanzmatrix;
}
.
.

void Adjazenzmatrix::berechneEigenschaften() {
wegmatrix = new Adjazenzmatrix();
distanzmatrix = new Adjazenzmatrix();
.
.
.
}

Did I understand it right, that every time when I execute
berechneEigenschaften() wegmatrix and distanzmatrix produce a memory
leak? Do I have to call "delete" already in berechneEigenschaften() or
will these objects become destroyed after berechneEigenschaften()'s
execution is finished?


Markus

 

[benben]

Hello,

I have a class Adjazenzmatrix with the following method and destructor:



private:
Adjazenzmatrix *wegmatrix;
Adjazenzmatrix *distanzmatrix;
.
.
.
Adjazenzmatrix::~Adjazenzmatrix() {
delete wegmatrix;
delete distanzmatrix;
}
.
.

void Adjazenzmatrix::berechneEigenschaften() {
wegmatrix = new Adjazenzmatrix();
distanzmatrix = new Adjazenzmatrix();
.
.
.
}

Did I understand it right, that every time when I execute
berechneEigenschaften() wegmatrix and distanzmatrix produce a memory
leak? Do I have to call "delete" already in berechneEigenschaften() or
will these objects become destroyed after berechneEigenschaften()'s
execution is finished?

Correct! Everytime adjazenzmatrix::berechneEigenschaften is invoked it
gives rise to a potential memory leak. Objects that are allocated
throught the operator new will NOT be automatically reclaimed (unless
you are running on a platforms that performs automatic garbage collection.)

There is no particular reason why the inner member objects should be
dynamically allocated. It would be much easier and simpler to just write

class Adjazenzmatrix
{

Adjazenzmatrix wegmatrix;
Adjazenzmatrix distanzmatrix;

// ...
};

Markus

Regards,
Ben

 

[Markus Pitha]

Hello,

understood, thanks for your answer.



Markus

 

[James Kanze]

I have a class Adjazenzmatrix with the following method and destructor:

private:
Adjazenzmatrix *wegmatrix;
Adjazenzmatrix *distanzmatrix;
.
.
.
Adjazenzmatrix::~Adjazenzmatrix() {
delete wegmatrix;
delete distanzmatrix;
}
.
.

void Adjazenzmatrix::berechneEigenschaften() {
wegmatrix = new Adjazenzmatrix();
distanzmatrix = new Adjazenzmatrix();
.
.
.
}

Did I understand it right, that every time when I execute
berechneEigenschaften() wegmatrix and distanzmatrix produce a memory
leak?

Maybe. If you only call berechneEigenschaften once, it's OK.
If you never call it, you delete uninitialized
pointers---undefined behavior.

Without more knowledge of the domain, I can't be sure, but I
wonder if wegmatrix and distanzmatrix shouldn't be values, and
not pointers. If they must be pointers, this might be a job for
boost::scoped_ptr (but watch out for the copy constructor), or
some sort of intelligent pointer which does deep copy. (From
what little I know, matrices are normally values, without
identity, so should be treated as such. Which means that
they're practically never dynamically allocated.)

Do I have to call "delete" already in berechneEigenschaften()
or will these objects become destroyed after
berechneEigenschaften()'s execution is finished?

Objects allocated with new are only destroyed by delete. If you
only need them in berechneEigenschaften, then they should
probably be local variables of the function, not members at all.
Something like:

void Adjazenzmatrix::berechneEigenschaften()
{
Adjazenzmatrix wegmatrix ;
Adjazenzmatrix distanzmatrix ;
// ...
}

In general, you should only use dynamic allocation when
necessary.

 

[Markus Pitha]

Hello,

Objects allocated with new are only destroyed by delete. If you
only need them in berechneEigenschaften, then they should
probably be local variables of the function, not members at all.

Actually I also need it out of this method and I'm not sure at the
moment why I used them as pointers. When my program is finished, I will
examine my code more closely again.


Markus

 

[Kai-Uwe Bux]

Hello,

I have a class Adjazenzmatrix with the following method and destructor:



private:
Adjazenzmatrix *wegmatrix;
Adjazenzmatrix *distanzmatrix;
.
.
.
Adjazenzmatrix::~Adjazenzmatrix() {
delete wegmatrix;
delete distanzmatrix;
}
.
.

void Adjazenzmatrix::berechneEigenschaften() {
wegmatrix = new Adjazenzmatrix();
distanzmatrix = new Adjazenzmatrix();
.
.
.
}

Did I understand it right, that every time when I execute
berechneEigenschaften() wegmatrix and distanzmatrix produce a memory
leak?

Looks like it.

Do I have to call "delete" already in berechneEigenschaften()

That will leave the two member variables dangling.

or
will these objects become destroyed after berechneEigenschaften()'s
execution is finished?

They will not be destroyed.


Your design looks very fishy. Why does a matrix have two pointers to
matrices as members? Effectively, you are defining a binary tree of
matrices here (although that is not reflected in the name of the class).
That just doesn't look right.

What are you trying to do? I have the feeling that you might be stuck
because you took a wrong turn a while ago and now you look for local
remedies to a global problem.


Best

Kai-Uwe Bux

 

[Markus Pitha]

Hello,

Your design looks very fishy. Why does a matrix have two pointers to
matrices as members? Effectively, you are defining a binary tree of
matrices here (although that is not reflected in the name of the class).
That just doesn't look right.

Yes, in the beginning I made some design mistakes.I thought about a MVC
concept, but now it became something different. I have all data
management in Adjazenzmatrix as well as every calculations. My
controller prepares the data from Adjazenzmatrix for the GUI and the GUI
class displays it.
And you are right. The name of the class "Adjazenzmatrix" is misleading
now. I will rename it after my program is finished.


Markus

 

[James Kanze]

Actually I also need it out of this method and I'm not sure at the
moment why I used them as pointers. When my program is finished, I will
examine my code more closely again.

I fear that that (and your response to Kai-Uwe) have it
backwards. These are things you have to do before even starting
to write the code. Define carefully what the class is to do,
and don't write code until you have that done.

As Kai-Uwe pointed out, if a class X has pointers to other class
X, then you have effectively defined a graph. Which may not be
what you meant to do.

 

[benben]

Correct! Everytime adjazenzmatrix::berechneEigenschaften is invoked it
gives rise to a potential memory leak. Objects that are allocated
throught the operator new will NOT be automatically reclaimed (unless
you are running on a platforms that performs automatic garbage collection.)

There is no particular reason why the inner member objects should be
dynamically allocated. It would be much easier and simpler to just write

class Adjazenzmatrix
{

Adjazenzmatrix wegmatrix;
Adjazenzmatrix distanzmatrix;

// ...
};

Oops! I didn't realise the members are of the same type as the class
being defined. My bad! The above will simply not compile.

Regards,
Ben

Regards,
Ben