Class design when one class contains an instance of another class

Question

I give the following example to illustrate my question. Suppose Abc is a class that will use another class (Def). Assume Def is a large class with many class members (expensive for copying), it make more sense to let the pointer to a Def object be part of Abc class. Then when implementing Abc class function do1, it can refers to Def object via its pointer pDef.

class Abc
{
  public:
        Abc(Def &def)
        {
           pDef = &def;
        }
        ~Abc()
        {
         }
        void do1();
   private:
        Def *pDef;

}

However, if I implement class Abc in this way, I will have the risk that the Def pointer might be invalid. Any ideas on improving this class design? One idea I have is to use share pointer:

 class Abc
    {
      public:
            Abc(boost::shared_ptr<Def> def)
            {
               pDef = def;
            }
            ~Abc()
            {
             }
            void do1();
       private:
            boost::shared_ptr<Def> pDef;

    }

EDIT: I want to make it clear that my main purpose is to avoid the expensive copy operation when constructing class Abc. For me using the shared pointer and James Adkison's solution (accepted answer) can both solve the problem. But the effect on the expensive Def object may be different. Using shared pointer will keep many copies of the object while using James Adkison's solution will only keep one copy of Def object.

Does `pDef` *really* have to be a pointer? And don't look at the smart pointers as simply self-deleting pointers and a way of faking garbage collection, look at them from an *ownership* perspective: Do your `Abc` instances share *ownership* of the `Def` instance or not? Also note that maybe you should be using `weak_ptr` instead? — Some programmer dude, Jul 05 '16 at 13:28
Did you have a look at http://stackoverflow.com/questions/892133/should-i-prefer-pointers-or-references-in-member-data? — Zafi, Jul 05 '16 at 13:29
Why can't `Abc` own `pDef` and use move and copy semantics as appropriate? — James Adkison, Jul 05 '16 at 13:31
If you have access to C++11, I would look into accepting a unique_ptr in the ctor so that you know you will own the memory. If you're not allowed to be the sole owner, use a `shared_ptr`. With raw pointers it's all about contracts (preconditions and postconditions) for managing lifetime, which is usually really difficult to puzzle out and notoriously ambiguous. — AndyG, Jul 05 '16 at 13:32
"Assume Def is a large class with many class members" -- Does this mean _data members_? If you're implying the class is expensive to copy just explicitly state it. — James Adkison, Jul 05 '16 at 13:33
@James Adkison using move seems to be a good solution, can you put your answer? Thanks. — feelfree, Jul 05 '16 at 13:38

James Adkison · Accepted Answer · 2016-07-05T14:50:33.553

3

Assume Def is a large class with many class members

I assume this means that it is expensive to copy the Def class. However, with C++11 you could implement move semantics for Def and keep the Abc implementation simple and efficient.

Note: This answer is based on the assumption that the only reason the question uses any pointers (smart or otherwise) is as a way to avoid expensive copies.

Example Code

class Def
{
public:
    Def() {}
    Def(const Def& other) { /* ... */ } // Copy constructor even though it's expensive
    Def(Def&& other) { /* ... */ } // Move constructor for efficiency

    Def& operator=(const Def& other) { /* ... */ } // Copy assignment
    Def& operator=(Def&& other) { /* ... */ } // Move assignment
};

Now the Def class supports copy semantics, even though copy the class may be expensive. However, it also supports move semantics to allow efficient usage when a copy isn't required.

class Abc
{
public:
    Abc() {}

    Abc(const Def& def) : mDef(def) {} // Perform an expensive copy

    Abc(Def&& def) : mDef(std::move(def)) {} // Perform a move

    // Implement any other member functions which could accept Def
    // via copy or move

private:
    Def mDef;
};

I would only advocate using std::shared_ptr if the use case is actually to support shared ownership semantics.

edited Jul 05 '16 at 14:50

answered Jul 05 '16 at 13:49

James Adkison

9,412
2
29
43

1

I'd like to hear back from the OP why this answer was accepted. It seems that the real issue was not shared ownership, but rather having to construct a temporary `Def` object only to pass it to an `Abc` constructor. Because with this approach, there is a `1-to-1` relationship between `Abc` and `Def`, while in the shared pointer approach, there is a `1-to-many` relationship from `Def` to `Abc`. It wasn't entirely clear from the question what relationship the OP desired. – rwols Jul 05 '16 at 14:06
@rwols I agree with you that is unclear. This is why I stated my assumption about the OP only using pointers as a mechanism to avoid expensive copy operations (of course that assumption could have been wrong). – James Adkison Jul 05 '16 at 14:09
@rwols @ James Adkison Thanks for the discussion. My main problem is to avoid expensive copy operations. I will reorganize my questions to make it clear. – feelfree Jul 05 '16 at 14:28
@feelfree from the editted question I agree that James' answer is the solution :-) – rwols Jul 05 '16 at 14:48

score 1 · Answer 2 · edited Jul 05 '16 at 13:53

1

Why use a pointer if you can use a reference?

class Abc
{
public:
    Abc(Def &def) : pDef(def)
    {
    }

    void do1();
private:
    Def& pDef;
}

You aren't passing ownership, so there's no need for pointers.

Think of a reference like a pointer that can never be null.

edited Jul 05 '16 at 13:53

Serge Ballesta

143,923
11
122
252

answered Jul 05 '16 at 13:27

Ivan Rubinson

3,001
4
19
48

Having an reference data member prevents an auto generated copy assignment operator whereas a pointer doesn't. – James Adkison Jul 05 '16 at 13:42
Well, just write one. It beats leaking any day. – Ivan Rubinson Jul 05 '16 at 13:42
1

The OP's code doesn't guarantee a leak and neither protect against the `Abc` instance outliving the `def` instance passed in the constructor. – James Adkison Jul 05 '16 at 13:52
To guarantee that, he'd need to put def inside Abc. – Ivan Rubinson Jul 05 '16 at 13:53
Using a reference doesn't buy you anything, and it makes assignment non-intuitive. – juanchopanza Jul 05 '16 at 14:24
If you aren't using pointers then you can let RAII take care of object lifetime. Assignment is still fine if you know what you're doing. – Ivan Rubinson Jul 05 '16 at 14:29
1

Using pointers or references here has absolutely nothing to do with RAII. – juanchopanza Jul 05 '16 at 14:30

score 0 · Answer 3 · answered Jul 05 '16 at 13:32

0

This is what std::shared_ptr is for. If you use std::shared_ptr, the pointer will always be valid (as long as you follow all the very simple rules).

answered Jul 05 '16 at 13:32

Sam Varshavchik

114,536
5
94
148

Class design when one class contains an instance of another class

3 Answers3

Example Code