setting a pointer member variable correctly

Question

I have a class L simplified as follows :

class L
{
    private:
        C * _pc ;
    public:
        C * getc() const ; // getter
        void setc(const C * ipc); //setter
        ~L()
        {
            delete _pc ;
        }
};

where C is another class.

I also have a helper class CHelper simplified as follows :

class CHelper
{
    C _c ;
    CHelper(L & ic)
    {
        // 1st constructs _c (code omitted);
        // then sets ic's _pc pointer member variable :
        ic.setc(&_c);
    }
};

I feel that there will be a problem with the deletion of _pc somehow, without being sure. What about it ?

What are the flaws of such an approch ? How can I reach the same "functionality" (setting correctly a pointer member variable) with a correct approach ?

http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — Oliver Charlesworth, Sep 05 '16 at 12:38
There are indeed issues with your approach. First, when `CHelper` object is destroyed, so is the `_c` member. `_pc` will point to garbage then. Second, `delete _pc;` will attempt a second destruction for the same object. I would suggest looking into std smart pointers. — Adrian Roman, Sep 05 '16 at 12:38
Your example will not compile. Inside the class `L` the destructor has the name `~C`. That does not work. Also you should initialize the `_pc` to a nullptr at the construction of the class. As an alternative you can consider smart pointer like `shared_ptr` or `unique_ptr`. — Hayt, Sep 05 '16 at 12:40
@AdrianRoman so I should replace the raw pointer `C * _pc` by a smart one and it would solve my issue ? How could I do it ? — Olórin, Sep 05 '16 at 12:40
It would solve it if you are a little careful with the ownership. For example you could use a 'unique' pointer and pass the object around. You won't be able to initialize it like in your example. Look for example into `make_unique` to see how it's done. — Adrian Roman, Sep 05 '16 at 12:41
@AdrianRoman If I want to initialize it as in my example, then I should rather use `shared_ptr` ? — Olórin, Sep 05 '16 at 12:45
If you really want to use it like that, then you must remove the `delete _pc` and ensure that the CHelper object is not destroyed before any usage of the `_pc` is ceased. — Adrian Roman, Sep 05 '16 at 12:46
If you want to use shared_ptr, you'll have to use a shared_ptr not only in L, but also in CHelper. — Adrian Roman, Sep 05 '16 at 12:53

score 0 · Accepted Answer · answered Sep 05 '16 at 13:00

Your code currently doesn't compile. CHelper's constructor asks for a C object. The C class (probably) doesn't have the setc method. you probably meant CHelper(L& ic).

Anyway, If your CHelper object goes out of scope and is destroyed then the member variable _c is destroyed along with it. Leaving you with a dangling pointer in your L object. Not only that, but L currently also violates the rule of three/five.

If you have access to C++11 I would greatly suggest to replace the raw pointer _pc and _c with a std::shared_ptr if the CHelper class is necessary ( and thus removing the delete in that destructor) and otherwise a std::unique_ptr.

You was right, I corrected the code. `_c` si not a pointer though — Olórin, Sep 05 '16 at 13:08
@user10000100_u But for shared ptr management it is required. — Hatted Rooster, Sep 05 '16 at 13:09

score -3 · Answer 2 · answered Sep 05 '16 at 12:52

-3

Check if ipc parameter is allocating like an array or not, because you must use delete [] _pc for arrays.

answered Sep 05 '16 at 12:52

Edmundo Robles L

25
3

setting a pointer member variable correctly

2 Answers2