Why Does The destructor destroy the original object??

Hello,
this is also one of the weirdest problem that i came across and even you would agree to it..the code is

Code:

---


#include <string.h>
#include <iostream.h>

class str
{
        char *s;
        public:
                ~str()
                {
                        delete s;
                }
                void getdata(char*);
                void putdata();
                friend int xstrlen(str);

};
int xstrlen(str A)
{
 int l=0;
 while(*A.s)
 {
        l++;
        A.s++;
 }
 return l;
}

void str::getdata(char* p)
{
        s=new char[strlen(p)+1];
        strcpy(s,p);
}
void str::putdata()
{
        cout<<s<<endl;
}

int main(void)
{
 
 str A;
 A.getdata("string");
 A.putdata();

 xstrlen(A);

 A.putdata();


 return 0;
}

---

Ok the 2nd A.putdata() prints garbage because when we come out of xstrlen the destructor gets called and destroys the object. That's fine but why does it destroy the original A object even though i have used call by value?? :) can anybody answer this to me??

Quote:

---

That's fine but why does it destroy the original A object even though i have used call by value?

---

It does that precisely because of call/pass by value. The object named A in xstrlen() is a copy of the object named A in main().

Kindly indent your code properly.

ya that's what i mean to say. If it's copy that means the original object shouldn't get modified?then why is it getting modified?why is the original object getting destroyed?

Quote:

---

If it's copy that means the original object shouldn't get modified?

---

Yes.

Quote:

---

then why is it getting modified?why is the original object getting destroyed?

---

However, you have char* member, and this pointer is copied. So, even though the original str object is not modified, the null terminated string that it points to was modified.

In other words, you need to provide a copy constructor and copy assignment operator that performs deep copying. Incidentally, your destructor is wrong as it should delete[], not delete.

If you were not writing a string class, using an existing string class like std::string would be best.

Ok thanks laserlight but i m not aware about it can you please write that for me?pleaseeee :(
just give me the function(copy constructor) that will solve this problem.

Quote:

---

just give me the function(copy constructor) that will solve this problem.

---

You pretty much have the implementation in getdata().

This is not supposed to be my homework still why are you giving me answers in round about manner?

can anybody else help me out?and write a copy constructor which does the job?

Quote:

---

This is not supposed to be my homework still why are you giving me answers in round about manner?

---

You have made it your homework, so give it a try first.

It doesn't matter if it's homework or not, simply giving a solution will not help you. But I can help you get started.
A copy constructor is a constructor that takes a reference to an object of its own type to copy from. So in your case, it will look like:

Definition:
str(const str& rCopy);

Implementation:
str::str(const str& rCopy)

In the copy constructor, you have to make sure to create a copy of your string, just like getdata does.

Because Laserlight [rightly] tries to teach you to solve your own problems, not learn how to get them "off the net".

A copy constructor is needed when you have allocated data in the class that gets deallocated in the destructor, as you have seen.

The default copy constructor is essentially this:

Code:

---

X(const X &x)
{
    memcpy(this, &x, sizeof(X));
}

---

But you need to create a new string in your copy constructor, otherwise the original pointer to the string is destroyed in the destructor.

--
Mats

Ok is this solution correct??

Code:

---


#include <string.h>
#include <iostream.h>

class str
{
        char *s;
        public:
                str(){}
                ~str()
                {
                        delete []s;
                }
                str(str &p)
                {
                        memcpy(this,&s,sizeof(s));
                        s=new char[strlen(p.s)+1];
                }
                void getdata(char*);
                void putdata();
                friend int xstrlen(str);

};
int xstrlen(str A)
{
 int l=0;
 while(*A.s)
 {
        l++;
        A.s++;
 }
 return l;
}

void str::getdata(char* p)
{
  s=new char[strlen(p)+1];
  strcpy(s,p);

}
void str::putdata()
{
        cout<<s<<endl;
}

int main(void)
{
 str A;
 A.getdata("string");
 A.putdata();


 xstrlen(A);
 A.putdata();

 return 0;
}

---

if yes! then now could you please explain me what am i doing when i call the xstrlen()??the control passes to copy constructor?but why??that i dont know?now please anybody explain me!what exactly is happening?

1) No, it's not correct. You are copying uninitialized data into an uninitialized pointer.
You must first allocate data using new. Then you should copy p.s (the copy's string), and not s itself, because it belongs to your own object which is being constructed.
2) getdata should possible return the string, and not set it. Misleading name.
3) You're passing by value, which means a new object is created when passed to the function. Everytime you do a copy of an object, its copy constructor is called.

Code:

---

int xstrlen(str A)

---

Since the argument for xstrlen() is not a reference, it requires the compiler to make a copy of the incoming argument. And to do that, it needs to call the copy constructor.

--
Mats

As Elysia hinted, your copy constructor isn't correct:

Code:

---

                str(str &p)
                {
                        memcpy(this,&s,sizeof(s));
                        s=new char[strlen(p.s)+1];
                }

---

The red code is not needed [and incorrect]
The green code is correct, but you would probably need to do more than just allocate the data space.

--
Mats

ok fine..let me try more

Ok is this one now correct??

Code:

---

class str
{
        char *s;
        public:
                str(){}
                ~str()
                {
                        delete []s;
                }
                str(str &p)
                {
                        s=new char[strlen(p.s)+1];
                        strcpy(s,p.s);
                }
                void getdata(char*);
                void putdata();
                friend int xstrlen(str);

};

---

Looks good to me.

--
Mats

Yep, much better. You can try to use more descriptive names for your variables and rename getdata/putdata to more appropriate names, too, if you want something extra to do.

Ok last and final question..is what i am doing in this function is that i am allocating a new memory space for my string so that the "string" and *s point to different memory location so when the destructor frees the memory the original object doesn't get destroyed?
is this correct what i have understood?

Yes, you are correct. You're making a copy of the data, so each class has its own instance of the string.

Ok this is deep copying. Now what is shallow copying?that one which is there in getdata???right??

but in that also i allocated the memory so how come *s and "string" both point to the same location?:(

Quote:

---

Originally Posted by chottachatri

Ok this is deep copying. Now what is shallow copying?that one which is there in getdata???right??

---

Shallow copy means you're just copying pointers over, while deep copy means you copy the data pointed to by the pointers, as well.

Quote:

---

but in that also i allocated the memory so how come *s and "string" both point to the same location?:(

---

Huh? There is no "string." You forgot to give them a descriptive name.


Oh, I also see something else. Always put names for arguments in class definitions, as well.
For example:
void getdata(char* pStr);

You mean the same 2 lines i have written in getdata and in copy constructor but still 1 is deep copy and 1 is shallow copy??? :o :(

You mean the difference is just of the copy constructor??is it the root?

Quote:

---

Originally Posted by chottachatri

You mean the same 2 lines i have written in getdata and in copy constructor but still 1 is deep copy and 1 is shallow copy??? :o :(

You mean the difference is just of the copy constructor??is it the root?

---

deep and shallow copying refers to how you copy an object. getdata() is not copying an object, is it?

--
Mats

Quote:

---

Originally Posted by chottachatri

You mean the same 2 lines i have written in getdata and in copy constructor but still 1 is deep copy and 1 is shallow copy??? :o :(

---

No, both are deep copy. Look again.

Quote:

---

You mean the difference is just of the copy constructor??is it the root?

---

That makes no sense...

If both are deep copy then Elysia how come are they(object's pointer and the string) pointing to the same memory location?

They won't, unless you do something wrong.
Remember that there are two instances of the objects. Both of these two will not pointer to the same memory location, since you allocated more memory and copied the data over.

Now you'll also need to overload the assignment operator. Otherwise you'll have the original problem when you do this:

Code:

---

str a, b;
a = b;

---

Quote:

---

Originally Posted by chottachatri

If both are deep copy then Elysia how come are they(object's pointer and the string) pointing to the same memory location?

---

Huh?

if you call the putData() function, you get a copy of the string [which your existing code is not freeing], and the copy constructor creates a copy of the string. So neither is pointing to the string in the original object.

--
Mats

OK this time i got it for sure!! you mean to say when we do call by value for objects(xstrlen..in this case) the default copy constructor get's called and it copies only the address of the original object and doesn't allocate the memory?right??

and now what is anon talking about?why should be overload the assignment operator??the assignment operator is already overloaded isnt it??and i tried B=A as well as A=B but i am getting the right output only?

The assignment operator will ALSO perform a plain memcpy(). Since that ALSO leads to two objects pointing to the same actual memory, it leads to similar problems.

Obviously, the problem only happens when something attempts to change the contents of the pointer, or the pointer gets deallocated.

--
Mats

Quote:

---

Originally Posted by chottachatri

OK this time i got it for sure!! you mean to say when we do call by value for objects(xstrlen..in this case) the default copy constructor get's called and it copies only the address of the original object and doesn't allocate the memory?right??

---

Yes, the default copy constructor does a shallow copy. It simply copies the contents of the variables over to the new instance.

Quote:

---

and now what is anon talking about?why should be overload the assignment operator??the assignment operator is already overloaded isnt it??and i tried B=A as well as A=B but i am getting the right output only?

---

Yes, you get right output, but try this:

Code:

---

void foo(str& mystr)
{
        str tempstr;
        mystr = tempstr;
        mystr.putdata();
}

int main()
{
        str str2;
        str2.putdata();
        foo(str2);
        str2.putdata();
        return 0;
}

---

Btw, this
str str2;
str2.putdata();
Might actually crash your program. To avoid this, you would do well to create a constructor.
Otherwise just add str2.getdata(""). Try this and you will encounter a little problem. The problem anon refers to.

Quote:

---

you mean to say when we do call by value for objects(xstrlen..in this case) the default copy constructor get's called and it copies only the address of the original object and doesn't allocate the memory?

---

No, it copies the member variables of the original object. If your original object has pointer member variables, it copies those too. But as it only copies the pointers, the objects that the pointers point to are not copied.

Quote:

---

why should be overload the assignment operator??the assignment operator is already overloaded isnt it?

---

You need to define the copy assignment operator for the class to do deep copying as well (and destroy what's currently in the object).