1

I'm rather new to C++ (with Java background) and I'm trying to understand the principles and practices for class designing.

From what I've already read it seems to me that one should prefer having class members as objects to having them as pointers (or references). - Please correct me if this is somehow wrong.

Based on that here's my design (version 1):

class Outer {
    Some_type _member;
    Inner _inner;
};

class Inner {
    Some_type* _ptr_to_outer_member;
};

I am aware that the lifetime of referenced object (Outer._member) needs to be guaranteed to exceed the lifetime of referencing object (Inner) and its pointer _ptr_to_outer_member to avoid dangling pointer. This condition should be satisfied here by the fact that _inner is a composite of _outer and Outer._member is an object member therefore it is always initialized.

Now I certainly don't need Outer to be copyable. Even being movable is not necessarily required since it's not supposed to be moved after everything is set up - although it'd be handy to have it movable when constructing other objects. Anyway with this design when Outer is moved _inner._ptr_to_outer_member gets invalidated. - This can be prevented by disabling copy and move operations for Outer but it kind of feels like it's just working around a bad practice. There's a guideline which says to avoid storing references to stack allocated variables and this case seems to me like it could have similar reasons to be avoided.

An alternative I can see is to heap allocate _member like this (version 2):

class Outer {
    std::unique_ptr<Some_type> _member;
    Inner _inner;
};

class Inner {
    Some_type* _ptr_to_outer_member;
};

Outer is now movable and non-copyable which is what I want but it's for the cost of heap allocating _member which is against the guideline about preferring object members.

So it gets me to the question: Are there any guidelines on this? I mean something like to use pointer to object as a members when you want to share the object (as in design version 2) - this would probably make the most sense to me (unless I'm missing something). Or to just disable moving for such objects (as in design version 1) and deal with it as it's non-movable.

Or is there something fundamentally wrong with the examples?

Thanks in advance for any insights on this.

tonkeRotti
  • 23
  • 3
  • In `Inner` use a reference to `Outer` (not a pointer) this will stop accidental moving and copying. – Richard Critten Apr 05 '20 at 11:29
  • I would suggest understanding how and when to use smart pointers: https://stackoverflow.com/questions/106508/what-is-a-smart-pointer-and-when-should-i-use-one – MShakeG Apr 05 '20 at 11:31
  • Outer can be made movable and non-copyable without having a std::unique_ptr member variable. – Eljay Apr 05 '20 at 11:40
  • @MoShaikG Thanks for your suggestion! But I've already read a lot about smart pointers and their use and now I'm not sure what exactly you're referring to. Are you implying I should use shared_ptr? If so I don't think this is the case since there's a clear relationship between `Outer` and `Inner` and so I can manage their lifetimes - so no need for reference counting here. Or did I get you wrong? – tonkeRotti Apr 05 '20 at 12:13
  • It depends on the lifetimes of Outer and Inner. Are they created together? Are they destroyed together? Do Outer or Inner need to move during their lifetime? – rustyx Apr 05 '20 at 12:13
  • @Eljay yes it can when I implement move operations that will reset `Inner._ptr_to_outer_member` to point to new memory location of `Outer._member` after moving `Outer`, right? My question is more about design approaches so in this case - Is it worth it to take care of it by implementing move operations for the sake of having `_member` as object member instead of pointer? I mean the approach with unique_ptr feels more straightforward to me and so I would prefer it unless there's some serious drawback I don't see. – tonkeRotti Apr 05 '20 at 12:22
  • The only drawbacks are: Outer becomes non-copy-able unless you implement your own copy constructor and assignment operator -- which sounds like that's actually desirable in this scenario; _member is allocated on the heap -- which is only a concern if Outer is an ultra-high performance concern. – Eljay Apr 05 '20 at 12:32
  • @rustyx `Outer` is created before `Inner` is created and destroyed after `Inner` is destroyed. So `Outer._member` is guaranteed to be alive while `Inner` lives. However `Outer._inner` can be switched to another instance of `Inner` during `Outer`s lifetime. None of them really needs to be movable except for that movability is handy for dependency injection. – tonkeRotti Apr 05 '20 at 12:35
  • @Eljay Ok thanks! Maybe one additional question here: In what situations does heap allocating `Outer._member` means performance penalties? I mean is it more about creating and manipulating (moving, copying) of `Outer` object (this is where it makes sense to me) or is also about accessing `Outer._member` from within `_inner`? E.g. if `Outer._member` is a callable and it's to be invoked from within `_inner` - is performance of this also affected? My guess is it's not affected since it's just using pointer to `Some_type` object regardless of how `Outer._member` is allocated. – tonkeRotti Apr 05 '20 at 12:55
  • You'd have to profile your code in your use cases to answer those questions definitively. In my head, I think of heap allocations as being x100 more expensive than stack allocations. – Eljay Apr 05 '20 at 13:01

1 Answers1

0

There are some guidelines, but they are generic, like use unique_ptr for owning pointers.

For the rest the guideline is: keep it simple. How did it come to this hierarchy with such an inter-dependency? Could there be a better way to structure your application?

Bidirectional references are possible but you need need to maintain them yourself, or make Outer non-copyable and non-movable:

class Outer {
    Some_type member;
    std::unique_ptr<Inner> inner;
public:
    Outer() {}

    Outer(Outer const& that) = delete; // makes Outer not copyable/movable
    Outer& operator=(Outer const& that) = delete;

    /* The only way to update inner, also sets the back-reference */
    void setInner(std::unique_ptr<Inner> ptr) noexcept {
        inner = std::move(ptr);
        if (inner) {
            inner->ptr_to_outer_member = &member;
        }
    }
};

If Outer is movable, it should actively maintain all back-references to itself to keep them in-sync.

For example like this:

class Some_type {
};
class Inner {
    Some_type* ptr_to_outer_member;
    friend class Outer; // so that Outer may reach the private ptr_to_outer_member
};

class Outer {
    Some_type member;
    std::unique_ptr<Inner> inner; // owning pointer to Inner
public:
    Outer() noexcept {
    }
    Outer(Outer&& that) noexcept {
        *this = std::move(that);
    }
    Outer& operator=(Outer&& that) noexcept {
        member = std::move(that.member);
        setInner(std::move(that.inner));
        return *this;
    }
    /* The only way to update inner, also sets the back-reference */
    void setInner(std::unique_ptr<Inner> ptr) noexcept {
        inner = std::move(ptr);
        if (inner) {
            inner->ptr_to_outer_member = &member;
        }
    }
};

int main() {
    Outer o;
    o.setInner(std::make_unique<Inner>());
    Outer o2(std::move(o)); // OK, move-construction
    Outer o3;
    o3 = std::move(o2); // OK, move-assignment
    Outer o4(o3); // error: Outer is not copyable
}

Having back-references in a movable type almost always requires a custom copy/move-constructor. Which means we have to also keep the rule of 3/5/0 in mind. In this example unique_ptr saves us from having a custom destructor. That may not always be the case.

rustyx
  • 80,671
  • 25
  • 200
  • 267