It's possible and in your code it's not causing harm. But it's dangerous, because if you copy CDad, then the keys and pointers will be copied along. The objects that the pointers will point to, and the reference inside those objects, however, will remain the same. If the original CDad object then goes out of scope, the references in the objects referenced by the pointers are dangling, referencing no valid object anymore.
Maybe you can to reverse lifetimes: Create the keys on the heap, and the kids as normal members within the class. So if you copy dad, the kids are copied, but the keys are not. I think keys are immutable, so you can share the same key among multiple kids.
This brings to another point: If your keys are reasonable small (read: not huge) and immutable (so you don't have update anomalies if you change one key, but not the others), consider creating it not on the heap too - so they are automatically copied along too, and pass them to the kids when they need the key. You can make them normal pointers of the kids, but i think that's ugly, because the kid does not contain a key - but uses it. So a pointer/reference or a function parameter fits well, but not a "real" data member.
If you are going with the shared key and the keys-on-heap, you should use smart pointers - because you have to keep track of all kids and dads. If the last kid/dad goes out of scope, you have to delete the key again. You use boost::shared_ptr
for that:
class CCarKeys
{
public:
CCarKeys(const string& Name) : _Name(Name) {}
string _Name;
};
class CChild
{
public:
CChild(boost::shared_ptr<CCarKeys> const& CarKeys)
: _Name("Child"), _CarKeys(CarKeys) {}
string _Name;
boost::shared_ptr<CCarKeys> _CarKeys;
void TestHasKeys() {cout << "I got " << _CarKeys._Name << endl;}
};
class CDad
{
public:
// NOTE: null the kid pointers *if* you use raw pointers, so you can check whether
// a boy or girl is present. Without nulling them explicitly, they have
// indeterminate values. Beware. shared_ptr's however will automatically
// initialized to default "null" values
CDad() :
_Name("Dad"),
_HondaCarKeys(new CCarKeys("Honda keys")),
_ChevyCarKeys(new CCarKeys("Chevy keys")) {}
string _Name;
boost::shared_ptr<CCarKeys> _HondaCarKeys;
boost::shared_ptr<CCarKeys> _ChevyCarKeys;
// also use shared_ptr for the kids. try to avoid raw pointers.
boost::shared_ptr<CChild> _Boy;
boost::shared_otr<CChild> _Girl;
void MakeBoy() {_Boy.reset(new CChild(_HondaCarKeys));}
void MakeGirl() {_Girl.reset(new CChild(_ChevyCarKeys));}
};
// main can be used unchanged
Of course, you can avoid all this complexity by just making the CDad class non-copyable. Then you can use your original solution, just with making the kids use a shared_ptr and make the kids non-copyable too. Ideally, one should use a non-shared pointer, such as auto_ptr
, but that auto_ptr has some pitfalls too, which shared_ptr all avoids:
class CCarKeys
{
public:
CCarKeys(const string& Name) : _Name(Name) {}
string _Name;
};
class CChild
{
public:
CChild (CCarKeys& CarKeys)
: _Name("Child"), _CarKeys(CarKeys) {}
string _Name;
CCarKeys &_CarKeys;
void TestHasKeys() {cout << "I got " << _CarKeys._Name << endl;}
private:
CChild(CChild const&); // non-copyable
CChild & operator=(CChild const&); // non-assignable
};
class CDad
{
public:
CDad() :
_Name("Dad"),
_HondaCarKeys("Honda keys"),
_ChevyCarKeys("Chevy keys") {}
string _Name;
CCarKeys _HondaCarKeys;
CCarKeys _ChevyCarKeys;
// also use shared_ptr for the kids. try to avoid raw pointers.
boost::shared_ptr<CChild> _Boy;
boost::shared_otr<CChild> _Girl;
void MakeBoy() {_Boy.reset(new CChild(_HondaCarKeys));}
void MakeGirl() {_Girl.reset(new CChild(_ChevyCarKeys));}
private:
CDad(CDad const&); // non-copyable
CDad & operator=(CDad const&); // non-assignable
};
If i had to implement such a class hierarchy, i would go with that solution, or just drop the keys as members, and pass/create them when needed to the children. Some other notes about your code:
- Better drop the "_" from members or put them at the end or use some other notation. A name that begins with an underscore and is followed by an uppercase letter is reserved by C++ implementations (compiler, C++ std lib ...).
- I personally find it confusing to have member names and variables start with an Uppercase letter. I've seen it only very rarely. But this isn't much to care about, it's just about personal style.
- There is a famous rule (Zero-One-Infinity) that states when you got two things of something, you generally should be able to have arbitrary many things of that. So if you can have two children - why not have many of them? Two seems like an arbitrary choice. But it may have a good reason in your case - so ignore this when in your case it makes sense.