Why doesn't the class/object keep its internal data properly whenreturned from function?

[Rob]

I have a vector of a class type and when I create an object inside a
function and return that and add it to the vector, it doesn't properly
keep the data inside it. So I have a few questions:

1. Is this because it loses scope since it wasn't created with "new"?
2. If I do create it with new, but the vector holds objects not
pointers, will the vector's "delete" operator function still handle
deleting all those pointers?

CODE 1:

//Doing without "new"

SomeClass create(string name)
{
return SomeClass( name );
}

int main()
{
std::vector<SomeClass> p;
p.push_back( create( "Fred" ) );
}


CODE 2:

//Doing with "new"

SomeClass create(string name)
{
return *( new SomeClass( name ) );
}

int main()
{
//Will calling "delete p" also delete all the pointers created
for the data it holds?
std::vector<SomeClass> p;
p.push_back( create( "Fred" ) );
}

 

[Erik Wikström]

I have a vector of a class type and when I create an object inside a
function and return that and add it to the vector, it doesn't properly
keep the data inside it. So I have a few questions:

1. Is this because it loses scope since it wasn't created with "new"?

No, it is probably because you have not implemented the copy-constructor
correctly, but without seeing the code it is impossible to tell.

2. If I do create it with new, but the vector holds objects not
pointers, will the vector's "delete" operator function still handle
deleting all those pointers?

No, and especially not if you return a copy of the new object and then
looses all references to it (like you do below).

CODE 1:

//Doing without "new"

SomeClass create(string name)
{
return SomeClass( name );
}

int main()
{
std::vector<SomeClass> p;
p.push_back( create( "Fred" ) );
}


CODE 2:

//Doing with "new"

SomeClass create(string name)
{
return *( new SomeClass( name ) );
}

This creates a copy of the "newed" object which is then returned, the
"newed" object is then lost (since you have no pointers to it) which
means you are leaking memory.

 

[Pascal J. Bourguignon]

I have a vector of a class type and when I create an object inside a
function and return that and add it to the vector, it doesn't properly
keep the data inside it. So I have a few questions:

1. Is this because it loses scope since it wasn't created with "new"?
2. If I do create it with new, but the vector holds objects not
pointers, will the vector's "delete" operator function still handle
deleting all those pointers?

CODE 1:

//Doing without "new"

SomeClass create(string name)
{
return SomeClass( name );
}

int main()
{
std::vector<SomeClass> p;
p.push_back( create( "Fred" ) );
}


CODE 2:

//Doing with "new"

SomeClass create(string name)
{
return *( new SomeClass( name ) );
}

int main()
{
//Will calling "delete p" also delete all the pointers created
for the data it holds?
std::vector<SomeClass> p;
p.push_back( create( "Fred" ) );
}

Both should work well. The second will leak memory (because you're
taking copies of the objects instead of keeping the pointers).

Things go better when you present fully compilable code:


#include <string>
#include <iostream>
#include <vector>
using namespace std;

class SomeClass {
public:
string name;
SomeClass(string aName):name(aName){};
};

void dump(vector<SomeClass>& v){
vector<SomeClass>::iterator it;
int i;
for(i=0,it=v.begin();it!=v.end();i++,it++){
cout<<i<<": "<<it->name<<endl;
}
}


SomeClass create1(string name)
{
return SomeClass( name );
}

void code1(void)
{
std::vector<SomeClass> p;
p.push_back( create1( "Fred" ) );
p.push_back( create1( "Merc" ) );
dump(p);
}


SomeClass create2(string name)
{
return *( new SomeClass( name ) );
}

void code2(void)
{
std::vector<SomeClass> p;
p.push_back( create2( "Fred" ) );
p.push_back( create2( "Merc" ) );
dump(p);
}

int main(void){
code1();
code2();
return(0);
}

/*
-*- mode: compilation; default-directory: "~/src/tests-c++/" -*-
Compilation started at Mon Apr 28 19:01:04

p=test-create ; cd /home/pjb/src/tests-c++/ ; g++ -o $p ${p}.c++ && ./$p
0: Fred
1: Merc
0: Fred
1: Merc

Compilation finished at Mon Apr 28 19:01:04
*/

 

[Rob]

Things go better when you present fully compilable code:

Yes, sorry about that. My problem was that I was deleting stuff in the
destructor and I didn't realize that in calling the copy constructor,
it also calls the destructor too. So, do I have to create a separate
method (like "void destroy()" ) to release all memory instead of using
the destructor and then call it explicitly if I'm using the copy
constructor?

 

[Bo Persson]

Yes, sorry about that. My problem was that I was deleting stuff in
the destructor and I didn't realize that in calling the copy
constructor, it also calls the destructor too. So, do I have to
create a separate method (like "void destroy()" ) to release all
memory instead of using the destructor and then call it explicitly
if I'm using the copy constructor?

No, but you probably have to have the copy constructor actually copy
everything, including things pointed to.


Bo Persson

 

[Jim Langston]

Yes, sorry about that. My problem was that I was deleting stuff in the
destructor and I didn't realize that in calling the copy constructor,
it also calls the destructor too. So, do I have to create a separate
method (like "void destroy()" ) to release all memory instead of using
the destructor and then call it explicitly if I'm using the copy
constructor?

Not necessarily. You need a proper copy constructor. The copy constructor
should *copy* everything in the class. So that your copy has it's own copy
and it won't be effected by the destructor of the old object.

Since I really don't know what SomeClass is allocating I'll just throw up
some example of untested code.

class SomeClass
{
public:
SomeClass() { Foo = new int[100]; }
~SomeClass() { delete[] Foo; }
private:
int* Foo;
};

Now, that class is not complete. It needs to follow the rule of three. It
will need a copy constructor and an operator=. Otherwise when a copy is
made the old copy will delete Foo and effect both copies. So I need to do
something like: (Note: this code probably has a lot of bugs, just showing
concept).

class SomeClass
{
public:
SomeClass() { Foo = new int[100]; }
~SomeClass() { delete[] Foo; }
SomeClass( const SomeClass& rhs )
{
Foo = new int[100];
std::copy( rhs.Foo, Foo + 100, Foo );
}
SomeClass& operator=( SomeClass const& rhs )
{
delete[] Foo;
Foo = new int[100];
std::copy( rhs.Foo, Foo + 100, Foo );
return this;
}
private:
int* Foo;
};

The whole idea being that each instance of Foo keeps it's own resources.
Also note that if Foo was a std::vector<int> none of this would be necessary
as std::vector already has the copy constructor and operator= overloaded and
would be called in the default constructor and operator=.

Since you didn't post SomeClass I really don't know what you're doing in
there, what your allocating or how you're allocating it.

 

[James Kanze]

Yes, sorry about that. My problem was that I was deleting stuff in the
destructor and I didn't realize that in calling the copy constructor,
it also calls the destructor too.

It doesn't. But you're copying something; what is the lifetime
of the object you are copying?

So, do I have to create a separate method (like "void
destroy()" ) to release all memory instead of using the
destructor and then call it explicitly if I'm using the copy
constructor?

No. You have to define a copy constructor (and probably an
assignment operator) which works correctly.

What you really have to do, however, is acquire a copy of Scott
Meyers' "Effective C++" and read it.

 

[Pascal J. Bourguignon]

[...]
SomeClass( const SomeClass& rhs )
{
Foo = new int[100];
std::copy( rhs.Foo, Foo + 100, Foo );

you mean:
std::copy( rhs.Foo, rhs.Foo + 100, Foo );

}
SomeClass& operator=( SomeClass const& rhs )
{
delete[] Foo;
Foo = new int[100];
std::copy( rhs.Foo, Foo + 100, Foo );

you mean:
std::copy( rhs.Foo, rhs.Foo+100, Foo );

or perhaps:

std::copy( rhs.Foo, rhs.Foo+sizeof(Foo)/sizeof(Foo[0]), Foo );

[...]