Re: [boost] shifted_ptr review

From: Douglas Gregor (gregod_at_[hidden])
Date: 2003-09-29 11:17:37

On Wednesday 24 September 2003 09:44 am, Douglas Gregor wrote:
> The review of Philippe Bouchard's shifted_ptr library begins now and runs
> through Tuesday, September 30.

My review of the library follows. As always, this review is not to be confused 
with my role as review manager.
==========================================================

I was eager to test this library: I have an internal representation for which 
I've been very picky about separating allocation, using pointer typedefs, and 
downcasting through an abstraction layer with the idea that, someday, I'd 
just drop in a garbage-collecting pointer to do the hard work for me. My 
intention, of course, was to drop in shifted_ptr as my test.

I got lost along the way. From the documentation, I understand how to allocate 
an object to be placed in a shifted_ptr, and I infer that the semantics are a 
little like shared_ptr's semantics, but are they? More specifically:

        - What are the semantics for the various collector_traits options? I see 
rc_collector, gc_collector, and os_collector: what the semantics of these 
three collectors, and how do they compare to each other? To shared_ptr?
        - Can I upcast/downcast a shifted_ptr? I'm looking for something like 
static_pointer_cast/dynamic_pointer_cast but I don't see them. 
        - Can I float between raw pointers and shifted_ptrs for a particular object? 
This would be suicide with shared_ptr, but isn't so with many 
garbage-collected pointers or when the reference count is internal to the 
object.

Here's what I expected from "yet another smart pointer with different 
alternatives":

        - An interface similar to shared_ptr, or scoped_ptr, or intrusive_ptr. If 
parts of that interface are not/can not be implemented (say, 
dynamic_pointer_cast), it might be worth noting this in the documentation. 
This is one of the many trade-offs in choosing a smart pointer.

        - A clear description of the semantics of each operation. Doxygen does well 
enough [1] at presenting the interface, but with the description of functions 
or classes is as short as "Affector", it doesn't help. I was once told that 
tools such as Doxygen do an excellent job of making code look well-structured 
and well-documented, even if they aren't. [2] A simple heuristic: if Doxygen 
puts up more text describing the interface than you have describing the 
actual operation, there is a problem.

        - A section describing the interface & semantics (in near-layman's terms) of 
shifted_ptr vs. the alternatives already in Boost. Most important here is a 
clear guide that says when one should use shifted_ptr and when one should not 
use shifted_ptr. For instance, I think the use case I mentioned above is a 
prime candidate for shifted_ptr, because I control all of the allocation 
sites; if I didn't control all of them, it seems that shifted_ptr would be 
extremely dangerous for me to use (e.g., because I could stick a raw pointer 
allocated without (so) into a shifted_ptr).

        - A similar section to the above but for the semantics of the various 
collector_traits. Be warned that you're in policy-based smart-pointer land 
with collector_traits, which makes the specification of semantics much, much 
harder.

Overall, I vote to reject this library. The semantics of smart pointers can be 
very subtle, and the documentation inadequately conveyed the semantics such 
that I could not suitably evaluate the library itself. I hope to see this 
library again, but before then it needs documentation comparable to that of 
the existing smart_ptr library to be useful.

        Doug

[1] BoostBook is, of course, much better at this <g>. Seriously, I do find 
that Doxygen adds so much information to even a trivial member function that 
it obscures the important part of the documentation.

[2] The originator of the comment is John Field, who mentioned this just prior 
to my candidacy exam where I'd use Doxygen to help give an overview of a 
design. Perhaps it sunk in more because of the context, but I've found this 
to be very true with many projects. The Doxygen interface description is 
there, but there isn't much other substance.