Re: [boost] [paired ptr] Proposing a new smart pointer object for managing bidirectional relationships between objects

Subject: Re: [boost] [paired ptr] Proposing a new smart pointer object for managing bidirectional relationships between objects
From: Dan Walters (dan683_at_[hidden])
Date: 2010-05-03 10:13:19

Hi Rob,

Thank your for your feedback, i am finding it very useful.

> The idea is interesting, but I'm not sure whether I'd use it.  The example you provide is not compelling.  I'd like to see some real, useful examples to motivate the need for paired_ptr.

I will construct some better examples to prove its worth. There is
already another example within the source package.

> You asked in the document, so I'll mention that Boost doesn't use mixed case and doesn't prepend "C" to class names.  You should adjust your examples accordingly.

I will amend the example.

> None of the code appears to be formatted properly.

And I will amend the documentation.

> Your "Usage" section should use initialization rather than assignment for paired_cat and paired_dog.  Indeed, from the surrounding text, it would seem that calling set_owner() is the only way to indicate the owner and that means it is possible to not specify an owner.  It would be better if the constructor required an owner so the user is forced to initialize the instance with one.  Doing so will eliminate the "unfortunate" sentence.

I was using initialization however this does cause a warning c4355
inside the user defined class constructor. warning C4355: 'this' :
used in base member initializer list

I could disable the warning in the paired_ptr header as the usage is
safe however this is fairly bad practice as it would result in the
users project having the warning disabled, and probably against boost
guidelines. Thus I opted for an initialization function.

> The type is named "paired_ptr" so why does "pair" appear in member functions like "connect_pair" and "disconnect_pair?"

I take it that you are suggesting a more suitable name would be
"connect_to_paired_ptr"? That would be more descriptive.

> Looking at the "Callback Functions" section, it appears that constructor overloads accepting the owner plus either or both of the callback functions would be in order versus requiring individual function calls.

I will extend this function.

> The callbacks should be boost::functions for maximum flexibility.

The problem here comes where I am specifying member functions to be
called back. boost::function does not support member functions and
recommends std::bind1st functionality. This results in three options:

1. use member functions in the current manner.
2. use function objects using boost::function
3. use bind1st and support both at the cost of less tidy code.

There is a clear advantage in specifying member functions rather than
say, void callback_connect(owner_type* p_owner,
paired_ptr<owner_type,other_type>* p_paired_ptr) as it is much tidier
to program and removes the need of global functions or function
objects. On the other hand, Your point of using boost::function is
persuasive as function objects are not a bad alternative and will
extend the ability of callback to allow other functions rather than
just member functions. I have a feeling that implementing this will
mean the user implementation will be difficult. I will take a second
look.

> You have assumed the size of a member function pointer is the same as an ordinary pointer, but that's a false assumption.  It depends upon platform.

Good to know.

> Rather than a fat and skinny version of paired_ptr, perhaps the callbacks could be stored in a separately allocated object that is created only when needed.  Indeed, using SBO you could keep the size to just two pointers.  When there are no callbacks, use just the data members, but when there are callbacks, store everything in a separate allocation.  (If the two data members point to the same address, it will be the address of memory allocated on the free store to hold two object pointers and two member function pointers.  If they refer to different addresses, then there is no free store allocation and there are no callbacks.)

That is a very good suggestion for keeping size down, and keeping
paired_ptr objects with callback inter operable with paired_ptr
objects without callback.

> The callbacks should know the paired_ptr invoking them, because a legitimate reaction might be to modify the paired_ptr's state and using boost::function, the callback may not be a member function or there may be more than one in a UDT and the callback would need to disambiguate.

I have already added this functionality since posting here :) But
again, the callbacks cannot have the option of being either a normal
function or member function as the parameter lists are different.
Perhaps the correct decision is for callback functions to be static
member functions or regular functions via boost::function.

Thanks again for your review Rob, it has given me lots to think about!
I hope my comments help clarify some holes in my documentation?

Best wishes,

Dan