pointer to a class

A

andylcx

Hi all, I have a question about the code below. Any idea abuot that,
thanks in advance!

myclass.h
class myclass
{
public:
myclass();
~myclass();
void setdata();

private:
yclass *ypr;
};

myclass.cpp
include"myclass.h"
include"yclass.h"

myclass::myclass()
{
yclass *ypr=new yclass;
}

myclass::~myclass()
{
delete ypr;
}

void myclass::setdata()
{
yclass *newpr=new yclass;
ypr=newpr; //My question is here. If I delete the "*ypr" pointer in
the destructor function of myclass, does the "*newpr" pointer being
deleted automatically since both of them have the same address? the
newpr class object has the same lifetime as myclass(after it was
created), and I have no idea when it should be deleted.
}
 
J

John Dibling

My question is here. If I delete the "*ypr" pointer in
the destructor function of myclass, does the "*newpr" pointer being
deleted automatically since both of them have the same address?

They don't have the same address. myclass::yptr points to one instance
of a yclass, and newpr points to another instance of yclass. General
rule of thumb: you need 1 call to 'delete' for every call to 'new'.
You have 2 calls to 'new', but only 1 call to 'delete'. Memory leak.
the newpr class object has the same lifetime as myclass(after it was
created), and I have no idea when it should be deleted.

Do you really need to instantiate the yclass outside of myclass' ctor?
Just let myclass manage it's own resources.

Take care,

John Dibling
 
H

Howard

Hi all, I have a question about the code below. Any idea abuot that,
thanks in advance!

myclass.h
class myclass
{
public:
myclass();
~myclass();
void setdata();

private:
yclass *ypr;
};

myclass.cpp
include"myclass.h"
include"yclass.h"

myclass::myclass()
{
yclass *ypr=new yclass;

That should be just

ypr = new yclass;

As it is, you're declaring (and then leaking) new local variable, not
setting the value of the member.
}

myclass::~myclass()
{
delete ypr;
}

void myclass::setdata()
{
yclass *newpr=new yclass;
ypr=newpr; //My question is here. If I delete the "*ypr" pointer in
the destructor function of myclass, does the "*newpr" pointer being
deleted automatically since both of them have the same address? the
newpr class object has the same lifetime as myclass(after it was
created), and I have no idea when it should be deleted.
}

The variable newpr is not a "class object", it's a pointer. There is just
one object here, and it is deleted when *any* pointer to it is deleted. But
when this function ends, the local newpr pointer is not deleted, it goes out
of scope, and simply doesn't exist any more. The object it pointed to lives
on, and in this case is now pointed to by the ypr member pointer. (So, you
don't have to worry about calling delete for it.)

But...why are you using the temporary at all? Why not just "ypr = new
yclass;"?

However!!! Notice that whatever ypr pointed to before you ran this function
is now leaked (i.e., it never gets destroyed). If you want to change what
ypr points to, you need to first delete ypr, then assign it a new instance
of yclass.

A note on your style: generally, a function with a name like "setdata" would
be passed whatever data it is you want to "set". The name is somewhat
misleading, if what it's doing is creating a new yclass member.

Finally, read up on the "rule of three". Whenever you have a dynamic member
(a pointer) like this, it's likely that you'll need a copy constructor and
an assignment operator as well.

-Howard
 
H

Howard

John Dibling said:
They don't have the same address. myclass::yptr points to one instance
of a yclass, and newpr points to another instance of yclass. General
rule of thumb: you need 1 call to 'delete' for every call to 'new'.
You have 2 calls to 'new', but only 1 call to 'delete'. Memory leak.

Well, they do point to the same address after this code:
yclass *newpr=new yclass;
ypr=newpr;

But, the problem is that ypr already pointed to something *before* this
code, and *that* object was leaked by simply assigning it a new value,
without deleting the existing object first.

(Just wanted to clarify where the leak occurred.)

-Howard
 
V

Victor Bazarov

Hi all, I have a question about the code below. Any idea abuot that,
thanks in advance!

myclass.h
class myclass
{
public:
myclass();
~myclass();
void setdata();

private:
yclass *ypr;
};

myclass.cpp
include"myclass.h"
include"yclass.h"

myclass::myclass()
{
yclass *ypr=new yclass;

This is a HUGE mistake. You declare a _local_ variable 'ypr', which
_hides_ the member 'ypr'. You may think you are assigning the result
of using 'new' to the 'this->ypr', but you actually aren't. The
declaration of the member 'ypr' is in the class definition. Drop the
'yclass *' here if you actually want to assign to the member. Better
yet, use _initialisation_ instead (read about "initiliaser lists" in
your favourite C++ book).
}

myclass::~myclass()
{
delete ypr;
}

Clear violation of "the Rule of Three". Read about it in archives.
void myclass::setdata()
{
yclass *newpr=new yclass;
ypr=newpr; //My question is here. If I delete the "*ypr" pointer in
the destructor function of myclass, does the "*newpr" pointer being
deleted automatically since both of them have the same address?

They don't have the same address. The have the same value. The 'newpr'
local variable carries the value to 'ypr' and is then discarded. You
can forget 'newpr', it's superfluous anyway. You could simply write

ypr = new yclass;

here instead of the two statements.
> the
newpr class object has the same lifetime as myclass(after it was
created), and I have no idea when it should be deleted.

There is no such thing as "newpr class object". 'newpr' is but
a pointer that has the value equal to the address of a dynamically
allocated 'yclass' object. You don't need to delete 'newpr' since
its value is assigned (and therefore owned) by 'ypr' in this function.

Let me make one more comment. OK, two. First, you need to understand
pointers before you attempt dynamic memory management. How you do it
is up to you. I suggest exploring addresses of real objects created
statically or automatically, but not dynamically. Pass the pointers
to functions, pass objects to functions, query the addresses of those
objects and the contents. See more books (than you have already seen).
Second, you have a memory leak in the 'setdata' member function. The
'ypr' pointer's old value that used to holds the address of another
dynamically allocated object, is lost when you assign another value to
it. So, the dynamic 'yclass' object, which you allocated in the
constructor, is hanging around forever, and you can't do anything about
it because you don't have its address any more.

V
 
H

Howard

So what you have to do is delete the old object within setdata:

void myclass::setdata()
{
yclass *newpr=new yclass;
delete ypr;
ypr=newpr;
}

Or just

void myclass::setdata()
{
delete ypr;
ypr = new yclass;
}

No need for a temporary.

-Howard
 
R

Rolf Magnus

Hi all, I have a question about the code below. Any idea abuot that,
thanks in advance!

myclass.h
class myclass
{
public:
myclass();
~myclass();
void setdata();

private:
yclass *ypr;
};

myclass.cpp
include"myclass.h"
include"yclass.h"

This shouldn't compile, because yclass in unknown in myclass.h.
myclass::myclass()
{
yclass *ypr=new yclass;
}

This defines a new local pointer in the constructor and initializes it with
the address of a new dynamically allocated object. Then, the constructor
finishes, the pointer goes out of scope, and the dynamic object becomes a
memory leak, because you lost the only pointer to it.
Probably, you rather want something like:

myclass::myclass()
: ypr(new yclass)
{
}
myclass::~myclass()
{
delete ypr;
}

void myclass::setdata()
{
yclass *newpr=new yclass;
ypr=newpr; //My question is here. If I delete the "*ypr" pointer in
the destructor function of myclass, does the "*newpr" pointer being
deleted automatically since both of them have the same address? the
newpr class object has the same lifetime as myclass(after it was
created), and I have no idea when it should be deleted.
}

You seem to be confusing the pointers with the objects they point to. If you
change the constructor like I wrote above, ypr already points to an object
of class yclass. So what happens in setdata is the following:
You create a second object of class yclass and initialize newpr with it.
Then you let ypr point to that same new object. Then the function ends and
newpr (only the pointer itself, nothing else) gets out of scope. So ypr now
points to the newly created object. When your myclass object gets
destroyed, its destructor will delete that one.
However, there is still the object that ypr has been pointing to before
setdata was called. Since you overwrote ypr with the address of another
object, the old object is lost. You don't have any pointer to it anymore,
so you can never delete it anymore.
So what you have to do is delete the old object within setdata:

void myclass::setdata()
{
yclass *newpr=new yclass;
delete ypr;
ypr=newpr;
}
 
R

Rolf Magnus

Howard said:
Or just

void myclass::setdata()
{
delete ypr;
ypr = new yclass;
}

No need for a temporary.

Well, it depends on how robust you want your code. Think what happens if the
operator new or yclass's constructor throws an exception.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,774
Messages
2,569,596
Members
45,143
Latest member
DewittMill
Top