Composed object initialization

Question

This is somewhat of a broad question that seems to have no one true answer.

I've been confused about the initialization of composed objects for quite some time. I've been formally taught to supply getters and setters for all member data and to favor raw pointers to objects instead of automatic objects - this seems to contrast with what many people on Stack Overflow (such as this popular post) advise.

How, then, should I go about initializing object-composed objects?

This is the way I would attempt initialization using what I've learned in school:

class SmallObject1 {
public:
    SmallObject1() {};
};

class SmallObject2 {
    public:
        SmallObject2() {};
};

class BigObject {
    private:
        SmallObject1 *obj1;
        SmallObject2 *obj2;
        int field1;
        int field2;
    public:
        BigObject() {}
        BigObject(SmallObject1* obj1, SmallObject2* obj2, int field1, int field2) {
        // Assign values as you would expect
        }
        ~BigObject() {
            delete obj1;
            delete obj2;
        }
    // Apply getters and setters for ALL members here
};

int main() {
    // Create data for BigObject object
    SmallObject1 *obj1 = new SmallObject1();
    SmallObject2 *obj2 = new SmallObject2();
    int field1 = 1;
    int field2 = 2;

    // Using setters
    BigObject *bobj1 = new BigObject();
    // Set obj1, obj2, field1, field2 using setters

    // Using overloaded contructor
    BigObject *bobj2 = new BigObject(obj1, obj2, field1, field2);

    return 0;
}

This design is appealing because it's readable (to me). The fact that BigObject has pointers to its member objects makes it possible to initialize obj1 and obj2 after initialization. However, the dynamic memory could make the program more complicated and confusing down the road, thus ripe for memory leaks. Additionally, the use of getters and setters clutter up the class and may also make the member data too easy to access and mutate.

Is this actually bad practice? I often find times where I need to initialize a member object separately from its owner, which makes automatic objects unappealing. Additionally, I have considered letting larger objects construct their own member objects. This seems to make more sense from a security standpoint, but less sense from an object responsibility standpoint.

Answer 1

I've been formally taught to supply getters and setters for all member data and to favor raw pointers to objects instead of automatic objects

Unfortunately you were taught wrong.

There's absolutely no reason to favor raw pointers over any standard library constructs like std::vector<>, std::array<>, or if you need to std::unique_ptr<>, std::shared_ptr<>.

The most common culprit in buggy software is that (rolled your own) memory management exposes flaws, and even worse these are usually hard to debug.

Answer 2

I've been formally taught to supply getters and setters for all member data and to favor raw pointers to objects instead of automatic objects

Personally, I have no problem with having setters and getters for all data members. It is a good practice to have and can save a lot of grief especially if you venture into threads. In fact, many UML tools autogenerate them for you. You just need to be aware of what to return. In this particular example, don't return a raw pointer to SmallObject1 *. Return SmallObject1 * const instead.

The second part about

raw pointers

is done for educational purposes.

---

For your main question: the way you structure object storage depends on the larger design. Is BigObject the only class that will ever use SmallObject's? Then I would put them completely inside of the BigObject as private members and do all the memory management there. If SmallObject's are shared between different objects, and not necessarily of BigObject class, then I would do what you did. However, I would store the references or pointers to const to them and not delete them in the BigObject class's destructor - BigObject didn't allocate them, it thus shouldn't be deleting.

Answer 3

Consider the following code:

class SmallObj {
public:
  int i_;
  double j_;
  SmallObj(int i, double j) : i_(i), j_(j) {}
};

class A {
  SmallObj so_;
  int x_;
public:
  A(SmallObj so, int x) : so_(so), x_(x) {}
  int something();
  int sox() const { return so_.i_; }
};

class B {
  SmallObj* so_;
  int x_;
public:
  B(SmallObj* so, int x) : so_(so), x_(x) {}
  ~B() { delete so_; }
  int something();
  int sox() const { return so_->i_; }
};

int a1() {
  A mya(SmallObj(1, 42.), -1.);
  mya.something();
  return mya.sox();
}

int a2() {
  SmallObj so(1, 42.);
  A mya(so, -1.);
  mya.something();
  return mya.sox();
}

int b() {
  SmallObj* so = new SmallObj(1, 42.);
  B myb(so, -1.);
  myb.something();
  return myb.sox();
}

The disadvantages with approach 'A':

• our concrete use of SmallObject makes us dependent on its definition: we can't just forward declare it,
• our instance of SmallObject is unique to our instance (not shared),

The disadvantages to approach 'B' are several:

• we need to establish an ownership contract and make the user aware of it,
• a dynamic memory allocation must be performed before every B is created,
• indirection is required to access the members of this vital object,
• we must test for null pointers if we are to support your default constructor case,
• destruction requires a further dynamic memory call,

One of the arguments against using automatic objects is the cost of passing them by value.

This is dubious: in many cases of trivial automatic objects the compiler can optimize for this situation and initialize the sub-object in-line. If the constructor is trivial, it may even be able to do everything in one stack initialization.

Here is GCC's -O3 implementation of a1()

_Z2a1v:
.LFB11:
  .cfi_startproc
  .cfi_personality 0x3,__gxx_personality_v0
  subq  $40, %rsp      ; <<
  .cfi_def_cfa_offset 48
  movabsq $4631107791820423168, %rsi  ; <<
  movq  %rsp, %rdi     ; <<
  movq  %rsi, 8(%rsp)  ; <<
  movl  $1, (%rsp)     ; <<
  movl  $-1, 16(%rsp)  ; <<
  call  _ZN1A9somethingEv
  movl  (%rsp), %eax
  addq  $40, %rsp
  .cfi_def_cfa_offset 8
  ret
  .cfi_endproc

The highlighted (; <<) lines are the compiler doing the in-place construction of A and it's SmallObj sub-object in a single shot.

And a2() optimizes very similarly:

_Z2a2v:
.LFB12:
  .cfi_startproc
  .cfi_personality 0x3,__gxx_personality_v0
  subq  $40, %rsp
  .cfi_def_cfa_offset 48
  movabsq $4631107791820423168, %rcx
  movq  %rsp, %rdi
  movq  %rcx, 8(%rsp)
  movl  $1, (%rsp)
  movl  $-1, 16(%rsp)
  call  _ZN1A9somethingEv
  movl  (%rsp), %eax
  addq  $40, %rsp
  .cfi_def_cfa_offset 8
  ret
  .cfi_endproc

And there there's b():

_Z1bv:
.LFB16:
        .cfi_startproc
        .cfi_personality 0x3,__gxx_personality_v0
        .cfi_lsda 0x3,.LLSDA16
        pushq   %rbx
        .cfi_def_cfa_offset 16
        .cfi_offset 3, -16
        movl    $16, %edi
        subq    $16, %rsp
        .cfi_def_cfa_offset 32
.LEHB0:
        call    _Znwm
.LEHE0:
        movabsq $4631107791820423168, %rdx
        movl    $1, (%rax)
        movq    %rsp, %rdi
        movq    %rdx, 8(%rax)
        movq    %rax, (%rsp)
        movl    $-1, 8(%rsp)
.LEHB1:
        call    _ZN1B9somethingEv
.LEHE1:
        movq    (%rsp), %rdi
        movl    (%rdi), %ebx
        call    _ZdlPv
        addq    $16, %rsp
        .cfi_remember_state
        .cfi_def_cfa_offset 16
        movl    %ebx, %eax
        popq    %rbx
        .cfi_def_cfa_offset 8
        ret
.L6:
        .cfi_restore_state
.L3:
        movq    (%rsp), %rdi
        movq    %rax, %rbx
        call    _ZdlPv
        movq    %rbx, %rdi
.LEHB2:
        call    _Unwind_Resume
.LEHE2:
        .cfi_endproc

Clearly, in this case, we paid a heavy price to pass by pointer instead of value.

Now lets consider the following piece of code:

class A {
    SmallObj* so_;
public:
    A(SmallObj* so);
    ~A();
};

class B {
    Database* db_;
public:
    B(Database* db);
    ~B();
};

From the above code, what is your expectation of ownership of "SmallObj" in the constructor of A? And what is your expectation of ownership of "Database" in B? Do you intend to construct a unique database connection for every B you create?

To further answer your question of favoring raw pointers, we need look no further than the 2011 C++ standard which introduced the concepts of std::unique_ptr and std::shared_ptr to help resolve ownership ambiguity that has existed since Cs strdup() (returns a pointer to a copy of a string, remember to free).

There is a proposal before the standards committee to introduce an observer_ptr in C++17, which is a non-owning wrapper around a raw pointer.

Using these with your preferred approach introduces a lot of boiler plate:

auto so = std::make_unique<SmallObject>(1, 42.);
A a(std::move(so), -1);

We know here the a has ownership of the so instance we allocated, as we explicitly grant it ownership via std::move. But all that being explicit costs characters. Contrast with:

A a(SmallObject(1, 42.), -1);

or

SmallObject so(1, 4.2);
A a(so, -1);

So I think overall there is very little case for favoring raw pointers for small objects for composition. You should review your material leading you to the conclusion as it seems likely you have overlooked or misunderstood factors in a recommendation of when to use raw pointers.

Answer 4

Others have described optimization reasons, I'm now viewing it from a type / functional perspective. According to Stroustrup, 'It is the job of every constructor to establish the class invariant'. What is your class invariant here? It's important to know (and define!), otherwise you'll pollute your member functions with ifs to check whether the operation is valid - this is not much better than having no types at all. In the 90s we had classes like that, but nowadays we really hold on to the invariant definitions and want objects to be in a valid state all the time. (Functional programming goes a step further and tries to extract variable state from objects, so objects can be const.)

• If your class is valid iff you have these sub-objects, then have them as members, period.
• If you want to share SmallObjects amongst BigObjects, then you need pointers.
• If it's valid not to have a given SmallObject, but you don't need to share, you might consider std::optional<SmallObject> members. Optional usually allocates locally (vs. heap), thus you might benefit from cache locality.
• If you find it difficult to construct such an object, e.g., too many constructor parameters, then you have two orthogonal problems: construction and class members. Solve construction problems by introducing a builder class (Builder pattern). A usually feasible solution is just to have all the parameters of all the constructors as optional members.

Note that many of us who prefer functional style consider builder an anti-pattern and only use it for deserialization, if at all. The reason behind, it's very difficult to reason about a builer (what comes out, will it succeed, which constructor gets calles). If you have your two ints, that's just that: two ints. Your best bet is usually just to keep them in separate variables, then it's up to the compiler to do all kinds of optimization. I wouldn't be surprised if the pieces would miraculously just fall in piece and your ints would be constructed 'in place', therefore no copy is required later.

OTOH, if you find that the same parameters 'get bounded' (get their value) in many places beforehand others, then you might introduce a type for them. In this case, your two ints will be a type (preferably a struct). You might decide if you want to make it a base class of BigObject, a member, or just a separate class (you'll have to choose the third if you have multiple binding orders) - in either case, your constructor will now take the new class instead of the two ints. You might even consider deprecating your other constructor (the one taking two ints) as 1. the new object can be constructed easily, 2. it might be shared (e.g. when creating items in a loop). If you want to keep the old constructor, make one of them a delegate to the other.