Initializing array of pointers to an object in a class constructor

J

John

I need an dynamic array of pointers to MyObj in a class.
Am I doing this right?

class SomeClass
{
public: SomeClass();
~SomeClass();
MyObj **pMyObj; // pointer to pointer == pointer to array of
pointers
int iNumPointers; // counter
}

SomeClass::SomeClass()
{
iNumPointers = 123;

pMyObj = new MyObj*[iNumPointers]; // create array of iNumPointers
pointers to MyObj
for (int i = 0; i < iNumPointers; i++) pMyObj = new MyObj(); // init each
pointer object
}

SomeClass::~SomeClass()
{
for (int i = 0; i < iNumPointers; i++) delete pMyObj; // destroy each
object first
delete [] pMyObj; // and destroy array of pointers
}

Is there an easier/smarter way to the same thing to avoid destructors? e.g.
using std::vector?
 
F

Francesco S. Carta

I need an dynamic array of pointers to MyObj in a class.
Am I doing this right?

class SomeClass
{
public: SomeClass();
~SomeClass();
MyObj **pMyObj; // pointer to pointer == pointer to array of
pointers
int iNumPointers; // counter
}

SomeClass::SomeClass()
{
iNumPointers = 123;

pMyObj = new MyObj*[iNumPointers]; // create array of iNumPointers
pointers to MyObj
for (int i = 0; i< iNumPointers; i++) pMyObj = new MyObj(); // init each
pointer object
}

SomeClass::~SomeClass()
{
for (int i = 0; i< iNumPointers; i++) delete pMyObj; // destroy each
object first
delete [] pMyObj; // and destroy array of pointers
}

Is there an easier/smarter way to the same thing to avoid destructors? e.g.
using std::vector?


Yes, you just need to use some kind of smart pointer, make a search and
you'll find plenty of options.

Just as a side note, the next time you post code on a medium that could
wrap, either put single line comments on separate lines or use block
comments.

Cheers
 
J

John

Is there an easier/smarter way to the same thing to avoid destructors?
Yes, you just need to use some kind of smart pointer, make a search and
you'll find plenty of options.

can you show an short example how would you do the above with smart pointer
(preferably std::vector)? wrapping smart pointer in a class constructor is
kind-of abstract to me...
 
F

Francesco S. Carta

can you show an short example how would you do the above with smart pointer
(preferably std::vector)? wrapping smart pointer in a class constructor is
kind-of abstract to me...


Well, nothing really difficult, you would just replace your array with a
vector, then in the constructor of you container (as well as somewhere
else, if needed) you would push smart pointers into the vector, maybe
within a loop, just like when you have looped your array to assign the
raw pointers with the addresses of the newly created objects:

vec.push_back(smart_pointer(new obj));

where vec is a std::vector, obj is the name of the class you want to
store pointers of, and smart_pointer is the constructor of the smart
pointer you decide to use - and that's the truly important point: the
smart pointer that you choose.

Make a search for "std::auto_ptr with std::vector" and you will see what
I mean.
 
F

Francesco S. Carta

Christian Hackl ha scritto:


Actually, if I really understand what you try to accomplish, then this
solution is incorrect. Use the other one with shared_ptr! :) (Use
std::vector<MyObj> and just forget about pointers if MyObj is not
polymorphic.)

The decision about directly storing objects or just pointers to them
doesn't just depend on whether they're polymorphic or not, it does
depend on their size too - but in that case, even the choice of the
container (std::vector vs std::list, for example) should be pondered.

Hey, if I can dare suggesting, avoid feeding "pappa pronta" ;-)
 
J

John H.

SomeClass::SomeClass(int count) :
   vec(count, std::shared_ptr<MyObj>(new MyObjDerived))
{

}

I think this will create a bunch of pointers to the same object. If
instead he wants a bunch of pointers to distinct objects, he might
try:
SomeClass::SomeClass() :
myObjPtrs(123)
{
std::for_each(myObjPtrs.begin(),
myObjPtrs.end(),
[](std::shared_ptr<MyObj> & i) { i =
std::shared_ptr<MyObj>(new MyObjDerived); });
};
 
F

Francesco S. Carta

SomeClass::SomeClass(int count) :
vec(count, std::shared_ptr<MyObj>(new MyObjDerived))
{

}

I think this will create a bunch of pointers to the same object. If
instead he wants a bunch of pointers to distinct objects, he might
try:
SomeClass::SomeClass() :
myObjPtrs(123)
{
std::for_each(myObjPtrs.begin(),
myObjPtrs.end(),
[](std::shared_ptr<MyObj> & i) { i =
std::shared_ptr<MyObj>(new MyObjDerived); });
};

Eheheehh, just to make the OP's life easier throwing in lambda functions...

Not that I dislike the advice, on the contrary, but maybe explicitly
mentioning it would have been nice ;-)
 
J

John H.

SomeClass::SomeClass() :
   myObjPtrs(123)
{
     std::for_each(myObjPtrs.begin(),
                   myObjPtrs.end(),
                   [](std::shared_ptr<MyObj>  &  i) { i =
std::shared_ptr<MyObj>(new MyObjDerived); });
};

Eheheehh, just to make the OP's life easier throwing in lambda functions....


Your point is well taken. Another way to do the above:

SomeClass::SomeClass() :
myObjPtrs(123)
{
for(int i=0; i<123; i++)
{
myObjPtrs = std::shared_ptr<MyObj>(new MyObj);
}
}
 
J

James Kanze

John ha scritto:
class SomeClass
{
public:
SomeClass(int count);
private:
std::vector<std::vector<MyObj> > vec;
};

Except that he wanted pointers. He didn't say why, so we can't
make any real assumptions: most of the time, pointers are used
for navigation between entity objects, which aren't copiable, so
the vector of object type doesn't work.
Or, if you really need pointers to MyObj (for example if MyObj is a
polymorphic base class):
class SomeClass
{
public:
SomeClass(int count);

private:
std::vector<std::shared_ptr<MyObj> > vec;
};

Again, if the pointers are used for navigation, this is not
a good solution. std::shared_ptr is handing in some specific
cases, but it is definitely not a general solution.
SomeClass::SomeClass()
{
iNumPointers = 123;
pMyObj = new MyObj*[iNumPointers]; // create array of iNumPointers
pointers to MyObj
for (int i = 0; i < iNumPointers; i++) pMyObj = new MyObj(); // init each
pointer object
}


This, on the other hand, makes it look like the vector of
objects *is* the best solution, and that his use of pointers is
wrong.
If MyObj is not polymorphic:
SomeClass::SomeClass(int count) :
vec(count)
{
}

This supposes a default constructor, of course.
If it is polymorphic:
SomeClass::SomeClass(int count) :
vec(count, std::shared_ptr<MyObj>(new MyObjDerived))
{
}

If all of the objects do belong to SomeClass, yes.
No destructor needed at all.

But you might want to provide one anyway, for various reasons.
The default destructor is inline.
 
F

Francesco S. Carta

Francesco S. Carta ha scritto:
[...]
The decision about directly storing objects or just pointers to them
doesn't just depend on whether they're polymorphic or not, it does
depend on their size too

It is of course over-simplifying to make this decision depend on whether
the class is polymorphic. More generally, you do not (that is, you
cannot) put objects of non-copyable classes into a vector, and it is
usually not a good idea or outright impossible for an entity class to
support copying.

So if the OP's "MyObj" is actually "DatabaseConnection" or
"WindowFrame", then of course you store the objects as pointers.

If "MyObj" is a non-entity class, then I'd say that semantics dictate
whether to store the objects as pointers or not. If you conceptually
need a copy, then just store objects, and put performance-related tricks
(copy-on-write etc) into the implementation of MyObj instead of dealing
with ownership issues outside of the class.

Well, I was oversimplifying too - polymorphism and size aren't enough,
we must take in account other issues and you correctly pointed out them.
Yes, that's true.


The "pappa" was far from "pronta", though! :) I noticed that the pappa
the OP had already cooked for himself was quite wrong, so I had to do
something about it. It's not like he can copy and paste it into his own
code, but it is correct enough to serve as an example.

My bad, that statement was so sharp that it didn't really represent what
I meant. I was referring to the fact of pointing out the shared pointer
in particular, while my approach to this topic was to make the OP aware
of the issues and investigate about all the possible solutions - for
that reason, I pointed out one smart pointer which is well known to be
the wrong choice in these cases (i.e. std::auto_ptr).

[OT]
Are you the same Christian Hackl that runs on the bobsleighs?
Just out of curiosity :)
[/OT]
 
J

John

Well, nothing really difficult, you would just replace your array with a
vector, then in the constructor of you container (as well as somewhere
else, if needed) you would push smart pointers into the vector, maybe
within a loop, just like when you have looped your array to assign the raw
pointers with the addresses of the newly created objects:

vec.push_back(smart_pointer(new obj));

Being relatively new to smart pointers it took me quite some time to figure
out what you meant to say here.

That would look something like:

class SomeClass
{
public: SomeClass();

/*vector is auto-destroyed on class destroy*/
/*so is scoped_ptr as well */
std::vector<boost::scoped_ptr<MyObj> > pMyObj;
int iNumPointers;
}

SomeClass::SomeClass()
{
iNumPointers = 123;

// reserve custom number of uninitialized pointers
pMyObj.reserve(iNumPointers);

// init each ptr
for (int i = 0; i < iNumPointers; i++)
pMyObj = boost::scoped_ptr<MyObj> pMyObj(new MyObj);
}

....
access object:
pMyObj[0][0]->Property1....


pMyObj should be here until the destruction of SomeClass. But
boost::scoped_ptr will go out of scope on exit of SomeClass constructor
right?
 
J

John

Have to correct myself boost::scoped_ptr is non-copyable so no can do in
std::vector.

class SomeClass
{
public: SomeClass();
/*vector is auto-destroyed on class destroy*/
/*so is scoped_ptr as well */
std::vector<boost::shared_ptr<MyObj> > pMyObj;
int iNumPointers;
}

SomeClass::SomeClass()
{
iNumPointers = 123;

pMyObj.resize(iNumPointers, boost::shared_ptr<MyObj>(new MyObj));
}

access object:
pMyObj[0]->Property1....

and I think now shared_ptr will again be gone after SomeClass constructor...
 
F

Francesco S. Carta

Well, nothing really difficult, you would just replace your array with a
vector, then in the constructor of you container (as well as somewhere
else, if needed) you would push smart pointers into the vector, maybe
within a loop, just like when you have looped your array to assign the raw
pointers with the addresses of the newly created objects:

vec.push_back(smart_pointer(new obj));

Being relatively new to smart pointers it took me quite some time to figure
out what you meant to say here.

That would look something like:

class SomeClass
{
public: SomeClass();

/*vector is auto-destroyed on class destroy*/
/*so is scoped_ptr as well */
std::vector<boost::scoped_ptr<MyObj> > pMyObj;
int iNumPointers;
}

SomeClass::SomeClass()
{
iNumPointers = 123;

// reserve custom number of uninitialized pointers
pMyObj.reserve(iNumPointers);

// init each ptr
for (int i = 0; i< iNumPointers; i++)
pMyObj = boost::scoped_ptr<MyObj> pMyObj(new MyObj);
}

...
access object:
pMyObj[0][0]->Property1....


pMyObj should be here until the destruction of SomeClass. But
boost::scoped_ptr will go out of scope on exit of SomeClass constructor
right?


No, you're not there yet, there are quite a lot of things that look
strange in your code that I think you should start from scratch after
reading some docs more:

http://www.boost.org/doc/libs/1_39_0/libs/smart_ptr/scoped_ptr.htm

There it clearly says that you cannot use it with STL containers.

(I'm not that experienced with smart pointers either, just for the records)

See the other branches of this thread: you've been suggested to use
shared_ptr:
http://www.boost.org/doc/libs/1_39_0/libs/smart_ptr/shared_ptr.htm

Let's have a look and see if it can be really used for this purpose.
 
J

John

No, you're not there yet, there are quite a lot of things that look
strange in your code that I think you should start from scratch after
reading some docs more:
http://www.boost.org/doc/libs/1_39_0/libs/smart_ptr/scoped_ptr.htm

I figured out after posting that std::vector needs copyable smart pointer
and already corrected in my post above...
the later one seems to compile and work properly after initializing with
resize. The scopes also seem ok but... just checking to be sure.
 
F

Francesco S. Carta

I figured out after posting that std::vector needs copyable smart pointer
and already corrected in my post above...

Yes, I noticed shortly after having posted my reply, never mind ;-)
the later one seems to compile and work properly after initializing with
resize. The scopes also seem ok but... just checking to be sure.

Nay, that doesn't look fine for me - although it's almost completely
correct - I'll post a reply to the other message.
 
F

Francesco S. Carta

Have to correct myself boost::scoped_ptr is non-copyable so no can do in
std::vector.

class SomeClass
{
public: SomeClass();
/*vector is auto-destroyed on class destroy*/
/*so is scoped_ptr as well */
std::vector<boost::shared_ptr<MyObj> > pMyObj;
int iNumPointers;
}

SomeClass::SomeClass()
{
iNumPointers = 123;

pMyObj.resize(iNumPointers, boost::shared_ptr<MyObj>(new MyObj));

With the above your are creating a vector with 123 shared_ptr that point
all to one single dynamic instance of MyObj - that's not a new mistake
in this thread, you should have noticed.

Try the following code as it is, then try it replacing the SomeClass
constructor with your version and you'll see the difference - but maybe
lower down that 123 to limit the output and make it clearer:

-------
#include <iostream>
#include <boost/shared_ptr.hpp>
#include <vector>

using namespace std;

class MyObj {
public:
int data;
MyObj() : data(0) {
instance = all_instances;
++all_instances;
cout << "MyObj #" << instance << " created" << endl;
}
MyObj(const MyObj& other) : data(other.data) {
instance = all_instances;
++all_instances;
cout << "MyObj #" << instance << " copy created" << endl;
}
MyObj& operator=(const MyObj& other) {
data = other.data;
cout << "MyObj #" << instance << " copied data from #"
<< other.instance << endl;
return *this;
}
~MyObj() {
cout << "MyObj #" << instance << " deleted" << endl;
}
int id() {
return instance;
}
private:
int instance;
static int all_instances;
};

int MyObj::all_instances = 0;

class SomeClass {
public:
SomeClass();
std::vector<boost::shared_ptr<MyObj> > pMyObj;
};

SomeClass::SomeClass() {
pMyObj.push_back(boost::shared_ptr<MyObj>(new MyObj));
pMyObj.push_back(boost::shared_ptr<MyObj>(new MyObj));
}

int main() {

SomeClass sc2;

cout << "#" << sc2.pMyObj[0]->id()
<< " data == " << sc2.pMyObj[0]->data
<< endl;

cout << "#" << sc2.pMyObj[1]->id()
<< " data == " << sc2.pMyObj[1]->data
<< endl;

{
SomeClass sc;

sc.pMyObj[0]->data = 42;
sc.pMyObj[1]->data = 78;

cout << "#" << sc.pMyObj[0]->id()
<< " data == " << sc.pMyObj[0]->data
<< endl;

cout << "#" << sc.pMyObj[1]->id()
<< " data == " << sc.pMyObj[1]->data
<< endl;

sc2 = sc;
}

cout << "#" << sc2.pMyObj[0]->id()
<< " data == " << sc2.pMyObj[0]->data
<< endl;

cout << "#" << sc2.pMyObj[1]->id()
<< " data == " << sc2.pMyObj[1]->data
<< endl;

return 0;
}
 
F

Francesco S. Carta

With the above your are creating a vector with 123 shared_ptr that point
all to one single dynamic instance of MyObj - that's not a new mistake
in this thread, you should have noticed.

Try the following code as it is, then try it replacing the SomeClass
constructor with your version and you'll see the difference - but maybe
lower down that 123 to limit the output and make it clearer:

-------

[snip]

The following should be even more clear - notice how the first two MyObj
dynamic instances get deleted after the "sc2 = sc;" instruction:

-------
#include <iostream>
#include <boost/shared_ptr.hpp>
#include <vector>

using namespace std;

class MyObj {
public:
int data;
MyObj() : data(0) {
instance = all_instances;
++all_instances;
cout << "MyObj #" << instance << " created" << endl;
}
MyObj(const MyObj& other) : data(other.data) {
instance = all_instances;
++all_instances;
cout << "MyObj #" << instance << " copy created" << endl;
}
MyObj& operator=(const MyObj& other) {
data = other.data;
cout << "MyObj #" << instance << " copied data from #" <<
other.instance << endl;
return *this;
}
~MyObj() {
cout << "MyObj #" << instance << " deleted" << endl;
}
void print() {
cout << "MyObj #" << instance << " data == " << data << endl;
}
private:
int instance;
static int all_instances;
};

int MyObj::all_instances = 0;

class SomeClass {
public:
SomeClass();
std::vector<boost::shared_ptr<MyObj> > pMyObj;
void print() {
for(int i = 0; i < pMyObj.size(); ++i) {
pMyObj->print();
}
}
};

SomeClass::SomeClass() {
pMyObj.push_back(boost::shared_ptr<MyObj>(new MyObj));
pMyObj.push_back(boost::shared_ptr<MyObj>(new MyObj));
}

int main() {
cout << "Creating sc2..." << endl;
SomeClass sc2;

cout << endl;
cout << "Dumping sc2 vector content..." << endl;
sc2.print();

{
cout << endl;
cout << "Creating sc..." << endl;
SomeClass sc;

sc.pMyObj[0]->data = 42;
sc.pMyObj[1]->data = 78;

cout << endl;
cout << "Dumping sc vector content..." << endl;
sc.print();

cout << endl;
cout << "Setting sc2 = sc; ..." << endl;
sc2 = sc;

cout << endl;
cout << "sc going out of scope..." << endl;
}

cout << endl;
cout << "Dumping sc2 vector content..." << endl;
sc2.print();

cout << endl;
cout << "sc2 going out of scope..." << endl;

return 0;
}
 
J

John

Try the following code as it is, then try it replacing the SomeClass
constructor with your version and you'll see the difference - but maybe
lower down that 123 to limit the output and make it clearer:

[snip]

now... that's a very comprehensive way to say that resize will initialize
with single (identical) value from second param and initializing through
loop (or push_back-s in this case) will not ;)
but i got the point. thanks for the added effort to clarify this.

well, the progress is/was hard. after figuring out how to enable compiler
memory guard that checks if all memory is deallocated i'm confident to say
that this finally works as it should. and it looks way cleaner than the
original pointers version.

also did a couple of memory checks to see if it points to different objects
and it does now.
Just for your convenience, I must point out that you should read the
c.l.c++ FAQ - you'll learn a lot from there, also how to post code here to
ease anyone else's reviewing task ;-)

will do!
 
F

Francesco S. Carta

Uhm... this thread has 22 posts in my newsreader, GG shows only 15...
damn GG
 
F

Francesco S. Carta

Try the following code as it is, then try it replacing the SomeClass
constructor with your version and you'll see the difference - but maybe
lower down that 123 to limit the output and make it clearer:

[snip]

now... that's a very comprehensive way to say that resize will initialize
with single (identical) value from second param and initializing through
loop (or push_back-s in this case) will not ;)
but i got the point. thanks for the added effort to clarify this.

Uh, was that bit of an overkill? ;-)

Actually, the examples posted here here (in general) are for everybody's
better comprehension - future readers and myself included ;-)
well, the progress is/was hard. after figuring out how to enable compiler
memory guard that checks if all memory is deallocated i'm confident to say
that this finally works as it should. and it looks way cleaner than the
original pointers version.

also did a couple of memory checks to see if it points to different objects
and it does now.

You see, that's the difference. Taking advantage of your toolset for
checking things out is the "advanced way" version of my cout-debugging -
but cout-debugging is easier to illustrate and post in a newsgroup ;-)

Have good coding,
cheers,
Francesco
 

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

No members online now.

Forum statistics

Threads
473,982
Messages
2,570,189
Members
46,735
Latest member
HikmatRamazanov

Latest Threads

Top