tags:

views:

115

answers:

5

hello gurus,

I have a class MyClassA. In its constructur, I am passing the pointer to instance of class B. I have some very basic questions related to this.

(1) First thing , is the following code correct? ( the code that makes a shallow copy and the code in methodA())

MyClassA::MyClassA(B *b){
    this.b = b;
}

void MyClassA::methodA(){

    int i; 
    i = b.getFooValue();
    // Should I rather be using the arrow operator here??
    // i = b->getFooValue()
}

(2) I am guessing I don't need to worry about deleting memory for MyClassA.b in the destructor ~MyClassA() as it is not allocated. Am I right?

thanks

Update: Thank you all for your answers! MyclassA is only interested in accessing the methods of class B. It is not taking ownership of B.

+2  A: 
  1. You need the arrow operator since b is a pointer.
  2. Yes, unless the user of MyClassA expects to take the ownership of b. (You can't even be sure if b is a stack variable where delete-ing it will may the code crash.)

Why don't you use a smart pointer, or even simpler, a reference?

KennyTM
Thanks Kenny! For (1) , do you mean this.b = b should be "this->b = b" Or you meant "i = b->getFooValue() ..or both. Regd use of smart pointer, could you please illustrate how to use it? Thank you very much
memC
@memC: Both. As for smart pointer (e.g. `shared_ptr`), see http://www.boost.org/doc/libs/1_41_0/libs/smart_ptr/shared_ptr.htm
KennyTM
+1  A: 

First thing , is the following code correct? ( the code that makes a shallow copy and the code in methodA())

The answer depends upon who owns the responsibility of the B object's memory. If MyClassA is supposed just to store the pointer of A without holding the responsibility to delete it then it is fine. Otherwise, you need to do the deep copy.

I am guessing I don't need to worry about deleting memory for MyClassA.b in the destructor ~MyClassA() as it is not allocated. Am I right?

Again depends on how memory for B is allocated. Is it allocated on stack or heap? If from stack then you need not explicitly free it in destructor of MyClassA, otherwise you need to to delete it.

Naveen
thanks for a detailed comment. This is very useful
memC
+1  A: 

1) this.b = b;

Here you pass a pointer to an instance of B. As Mac notes, this should be:

this->b = b;

b.getFooValue();

This should be b->getFooValue(), because MyClassA::b is a pointer to B.

2) This depends of how you define what MyClassA::b is. If you specify (in code comments) that MyClassA takes over ownership over the B instance passed in MyClassA's constructor, then you'll need to delete b in MyClassA's destructor. If you specify that it only keeps a reference to b, without taking over the ownership, then you don't have to.

PS. Regrettably, in your example there is no way to make ownership explicit other than in code documentation.

Dimitri C.
It should be this->b = b; since this is a pointer.
Ashish
@Mac: You're right. This one slipped my eye.
Dimitri C.
+1  A: 

1) . It depends on the life time of the pointer to B. Make sure the when you call b->getFooValue(); b should be a valid pointer.

I will suggest use of initilization list and if you are only reading the value of the B object though it pointer then make it pointer to constant data.

MyClassA::MyClassA(const B *bObj) : b(bObj)
{}

2). As long as B is on the stack on need to delete it and if it is allocated to heap then it must be deleted by it the owner else you will have memory leak. You can use smart pointer to get rid of the problem.

Ashish
+1  A: 
MyClassA::MyClassA(B *b){
this.b = b;

}

should be:

MyClassA::MyClassA(B *b){
this->b = b;

}

because this is treated as a pointer.

Bryan
yeap, thank you!
memC