Hi, Whenever i have a class that contains references to other classes
i keep
end up doing the below. However it doesn't seem so safe or elegant.
How do the
pros initialise subobjects, any ideas to improve the below would be
welcome.

/.h file
class A
{
public:
A();
~A();
private:
SomeMethod();

obj1* pobj1;
obj2* pobj2;
obj3* pobj3;
obj4* pobj4;
};

/.cpp file

A::A()
{
pobj1 = new obj1;
pobj2 = new obj2;
pobj3 = new obj3;
pobj4 = new obj4;

}

A::SomeMethod()
{

pobj1->doSomething();
etc
}

A::~A()
{
delete pobj1;
delete pobj2;
delete pobj3;
delete pobj4;
}

tech wrote:
> Hi, Whenever i have a class that contains references to other classes
> i keep
> end up doing the below. However it doesn't seem so safe or elegant.
> How do the
> pros initialise subobjects, any ideas to improve the below would be
> welcome.
>
> /.h file
> class A
> {
> public:
> A();
> ~A();
> private:
> SomeMethod();
>
> obj1* pobj1;
> obj2* pobj2;
> obj3* pobj3;
> obj4* pobj4;
> };
>
> /.cpp file
>
> A::A()
> {
> pobj1 = new obj1;
> pobj2 = new obj2;
> pobj3 = new obj3;
> pobj4 = new obj4;
>
> }
>
> A::SomeMethod()
> {
>
> pobj1->doSomething();
> etc
> }
>
> A::~A()
> {
> delete pobj1;
> delete pobj2;
> delete pobj3;
> delete pobj4;
> }

Compare your code (which, BTW, doesn't adhere to "The Rule of Three",
read up on it) with this:

/.h file
class A
{
public:
A();
~A();
private:
SomeMethod();

obj1 myobj1;
obj2 myobj2;
obj3 myobj3;
obj4 myobj4;
};

/.cpp file

A::A()
: myobj1()
, myobj2()
, myobj3()
, myobj4()
{
}

A::SomeMethod()
{
myobj1.doSomething();
etc
}

A::~A()
{
}

-----------

If your object *owns* its subobjects, it probably is better if the data
members are represented by objects, not pointers.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

tech wrote:

> Hi, Whenever i have a class that contains references to other classes
> i keep
> end up doing the below. However it doesn't seem so safe or elegant.
> How do the
> pros initialise subobjects, any ideas to improve the below would be
> welcome.
>
> /.h file
> class A
> {
> public:
> A();
> ~A();
> private:
> SomeMethod();
>
> obj1* pobj1;
> obj2* pobj2;
> obj3* pobj3;
> obj4* pobj4;
> };
>
> /.cpp file
>
> A::A()
> {
> pobj1 = new obj1;
> pobj2 = new obj2;
> pobj3 = new obj3;
> pobj4 = new obj4;

If the last new throws, you leak the memory for the first three objects. If
you _really_ need pointers at all, consider using std::auto_ptr during
initialization:

A::A() {
std::auto_ptr< obj1 > dummy1 ( new obj1 );
std::auto_ptr< obj2 > dummy2 ( new obj2 );
std::auto_ptr< obj3 > dummy3 ( new obj3 );
std::auto_ptr< obj4 > dummy4 ( new obj4 );
pobj1 = dummy1;
pobj2 = dummy2;
pobj3 = dummy3;
pobj4 = dummy4;
}

>
> }
>
> A::SomeMethod()
> {
>
> pobj1->doSomething();
> etc
> }
>
> A::~A()
> {
> delete pobj1;
> delete pobj2;
> delete pobj3;
> delete pobj4;
> }

If you go with those pointers, you either need to make the assignment
operator and copy constructor private or implement them in some way that
does the RightThing(tm), whatever that would be in your case. The ones
generated by the compiler will _not_ do the right thing.


More importantly: why do you want pointers in the first place? You could
just do

class A {
type1 obj1;
type2 obj2;
...
};

Nothing in your post shows a genuine need for pointer members.


Best

Kai-Uwe Bux

Hi!

Kai-Uwe Bux schrieb:
> A::A() {
> std::auto_ptr< obj1 > dummy1 ( new obj1 );
> std::auto_ptr< obj2 > dummy2 ( new obj2 );
> std::auto_ptr< obj3 > dummy3 ( new obj3 );
> std::auto_ptr< obj4 > dummy4 ( new obj4 );
> pobj1 = dummy1;

You need to "release()" the object from the auto_ptr:
pobj1 = dummy1.release();

But it would be easier to just use auto_ptrs as members (if you _really_
need pointers after all):

class A {
//use const until you implement operator =
const std::auto_ptr<obj1> pobj1;
A();
};

A::A()
: pobj1(new obj1)
{}

Frank

>
> If the last new throws, you leak the memory for the first three objects. If
> you _really_ need pointers at all, consider using std::auto_ptr during
> initialization:
>
> A::A() {
> std::auto_ptr< obj1 > dummy1 ( new obj1 );
> std::auto_ptr< obj2 > dummy2 ( new obj2 );
> std::auto_ptr< obj3 > dummy3 ( new obj3 );
> std::auto_ptr< obj4 > dummy4 ( new obj4 );
> pobj1 = dummy1;
> pobj2 = dummy2;
> pobj3 = dummy3;
> pobj4 = dummy4;
> }


Actually, I could be seeing something correctly, but won't all your
pointers be destroyed as soon as you leave the scope of the
constructor? Don't you either have to explicitly release the auto_ptr
(i.e. dummy1.release()) after the assignment... of if you can,
declare your pointers in your class as auto_ptrs i.e.:

/.h file
class A
{
public:
A();
~A();
private:
SomeMethod();

std::auto_ptr< obj1 > myobj1;
std::auto_ptr< obj2 > myobj2;
std::auto_ptr< obj3 > myobj3;
std::auto_ptr< obj4 > myobj4;

};

A::A()
: myobj1(new obj1),
myobj2(new obj2),
myobj3(new obj3),
myobj4(new obj4)
{
}

Of course, they do not have to be auto_ptrs I guess... but using some
type of smart pointer in the declaration may make your life easier
than just using raw pointers.

bjeremy wrote:

>
>>
>> If the last new throws, you leak the memory for the first three objects.
>> If you _really_ need pointers at all, consider using std::auto_ptr during
>> initialization:
>>
>> A::A() {
>> std::auto_ptr< obj1 > dummy1 ( new obj1 );
>> std::auto_ptr< obj2 > dummy2 ( new obj2 );
>> std::auto_ptr< obj3 > dummy3 ( new obj3 );
>> std::auto_ptr< obj4 > dummy4 ( new obj4 );
>> pobj1 = dummy1;
>> pobj2 = dummy2;
>> pobj3 = dummy3;
>> pobj4 = dummy4;
>> }
>
>
> Actually, I could be seeing something correctly, but won't all your
> pointers be destroyed as soon as you leave the scope of the
> constructor?

Oops. That should be

pobj1 = dummy1.release();
...

> Don't you either have to explicitly release the auto_ptr
> (i.e. dummy1.release()) after the assignment... of if you can,
> declare your pointers in your class as auto_ptrs i.e.:
>
> /.h file
> class A
> {
> public:
> A();
> ~A();
> private:
> SomeMethod();
>
> std::auto_ptr< obj1 > myobj1;
> std::auto_ptr< obj2 > myobj2;
> std::auto_ptr< obj3 > myobj3;
> std::auto_ptr< obj4 > myobj4;
>
> };
>
> A::A()
> : myobj1(new obj1),
> myobj2(new obj2),
> myobj3(new obj3),
> myobj4(new obj4)
> {
> }
>
> Of course, they do not have to be auto_ptrs I guess... but using some
> type of smart pointer in the declaration may make your life easier
> than just using raw pointers.

Yes.


Thanks

Kai-Uwe Bux

"tech" <naumansulaiman@googlemail.com> wrote in message
news:fc6f277b-31ca-45b7-a8ea-b86b2b5101d2@p25g2000hsf.googlegroups.com...
> Hi, Whenever i have a class that contains references to other classes
> i keep
> end up doing the below. However it doesn't seem so safe or elegant.
> How do the
> pros initialise subobjects, any ideas to improve the below would be
> welcome.

__________________________________________________ _________
#include <memory>
#include <cstdio>

#define PRINTF_THIS(mp_this, mp_name, mp_func) ( \
std::printf("(%p)->" # mp_name "::" # mp_func "\n", \
(void*)(mp_this)) \
)

struct obj1 {
obj1() { PRINTF_THIS(this, obj1, obj1()); }
~obj1() { PRINTF_THIS(this, obj1, ~obj1()); }
};

struct obj2 {
obj2() { PRINTF_THIS(this, obj2, obj2()); }
~obj2() { PRINTF_THIS(this, obj2, ~obj2()); }
};

struct obj3 {
obj3() { PRINTF_THIS(this, obj3, obj3()); }
~obj3() { PRINTF_THIS(this, obj3, ~obj3()); }
};

struct obj4 {
obj4() { PRINTF_THIS(this, obj4, obj4()); }
~obj4() { PRINTF_THIS(this, obj4, ~obj4()); }
};

class not_copyable {
not_copyable(not_copyable const&);
not_copyable const& operator =(not_copyable const&);
protected:
not_copyable() {}
~not_copyable() {}
};

class A : private not_copyable {
std::auto_ptr<obj1> m_obj1;
std::auto_ptr<obj2> m_obj2;
std::auto_ptr<obj3> m_obj3;
std::auto_ptr<obj4> m_obj4;

public:
A()
: m_obj1(new obj1()),
m_obj2(new obj2()),
m_obj3(new obj3()),
m_obj4(new obj4()) {
PRINTF_THIS(this, A, A());
}

~A() { PRINTF_THIS(this, A, ~A()); }
};

int main() {
{
A a;
}
std::puts("\n\n_____\npress <ENTER> to exit...");
std::getchar();
return 0;
}

__________________________________________________ _________



Does that do what you want?

Thanks for all the replies, one further question

if i don't usse pointers but object members instead i need to include
the header files rather than forward declare. Isn't this an argument
against having member objects?