OOP Design Question (MFC C++ implementation)

Question

I have a GUI to interact with the user, but I have an OOP design problem with this.

Through a dialog the user specifies CDiscreteDistributions and they are stored in a std::vector<CDiscreteDistribution*> in the MyAppDoc class for serialization. Through another dialog the user chooses a type of CDistribution for a particular CParameter. CDiscreteDistribution, CConstantDistribution, and CContinuousDistribution inherit CDistribution, and CParameter has a polymorphic pointer to a CDistribution member variable. MyAppDoc has a container class of CParameter. Thus the CDiscreteDistributions are pointed two twice, but only exist once.

In summary, MyAppDoc has

1. std::vector<CDiscreteDistribution*>
2. CContainer which has many CParameter which have
   • CDistribution* which can point to one of
     • CDiscreteDistribution which is one of the CDiscreteDistribution*s stored above
     • CConstantDistribution created/destroyed by CParameter
     • CContinuousDistribution created/destroyed by CParameter

This design pattern is causing me various nightmares in porting the app to use shared_ptr due to double deletes and serialization (boost). Should one of the pointers to CDiscreteDistribution be a weak_ptr? If so where should own the pointer?

Thanks for any help!

---

EDIT: I re-thought the reasoning for having std::vector<CDiscreteDistribution*> and it was just to avoid copying the vector into and out of the GUI. But the objects are quite small, and so I've broken the link between them and suffer the minor performance implications. Now MyAppDoc has:

1. std::vector<CDiscreteDistribution>
2. CContainer which has many CParameter which have
   • CDistribution* which can point to one of
     • CDiscreteDistribution created/destroyed by CParameter, copied from one of the CDiscreteDistributions stored above
     • CConstantDistribution created/destroyed by CParameter
     • CContinuousDistribution created/destroyed by CParameter

I think part of the problem was boost::serialization made two shared_ptrs for each CDiscreteDistribution that weren't aware of each other's existence. Now the only issue is backwards compatibility to files created with the previous versions.

I figure this 'solution' is actually just avoiding a proper design!

Answer 1

The question is described not enough to understand the full situation, complications and exact problem, but in general -

• I assume you want to use shared_ptr to not have to manually delete() objects

• If so, see if you can solve it by not using shared_ptr, but rather using boost::ptr_vector instead of a vector of raw pointers; the ptr_vector will then handle memory management for you.

I'm not even sure what the shared_ptr would bring you - it's quite obvious, I'd say from my limited understanding of the situation, that the Doc owns the CDiscreteDistribution objects. Whoever owns the other two types of Distributions is responsible for deleting them; this can be done though a shared_ptr or otherwise. (you say 'locally instanced' but that doesn't mean much - are they instantiated on the heap or the stack? What is their lifetime? Why is their lifetime different from the DiscreteDistribution objects? What is 'local' - local to what?)

Answer 2

I agree with Roel that the question is not fully specified. But having done several extensive conversions from raw pointers to shared_ptr, I can give you a little advice.

1. weak_ptr should not be necessary unless you have circular dependencies. In other words, if an object A has a shared_ptr to object B and object B has a shared_ptr back to object A. In this case, it's impossible for the reference count of either pointer to go to 0, so you would either need to manually intervene to break the cycle, or use weak_ptr to designate one side as dependent.
2. I'm confused why you have issues with double deletion when using shared_ptr. One of the advantages of smart pointers in general is that you don't have to actually delete them. If you've converted all your raw pointers to shared_ptr, you shouldn't have this problem. If your CConstantDistribution and CContinuousDistribution objects are actually locals rather than allocated with new (I can't tell 100% if this is the case from your description), you can make them shared_ptr objects that are initialized in your constructor, if you can change the app's code. This would allow you to make your std::vector<CDiscreteDistribution*> a container of shared_ptr to CDiscreteDistribution instead. At that point, you shouldn't have to worry about deleting those objects at all, unless you have circular references as described above. Even if you do, you'd have converted a double-delete crash to a memory leak, which is generally less bad.
3. Serialization can be tough. Since you've tagged the question with MFC, I'll assume you're using MFC serialization. I generally don't have problems when wrapping everything in shared_ptr when I serialize out to a file -- I just use .get() on the smart pointer object and serialize the resulting raw pointer. When I serialize in, I read the raw pointer from the serialized file and wrap it in the shared_ptr candy coating at that point. It's a little extra code in the serialization function, but it works.

If I've guessed inaccurately on some of your situation, feel free to add a comment. I'd be happy to help further if I can.