Re: a nasty accident of API design

[Chris Vine <chris cvine freeserve co uk>]

On Monday 28 November 2005 16:51, Murray Cumming wrote:
> > consider:
> >
> > 	Glib::RefPtr<Gtk::TreeStore> model = ListStore::create ();
> >
> > 	....
> >
> > 	model->clear();
> > 	model.clear();
> >
> > those last two lines both compile, but they have a completely different
> > effect. the first clears the model, the second "clears" the refptr.
> >
> > it seems to me that it would be wise to make sure that envelope classes
> > like RefPtr have no methods that are likely to also exist in the wrapped
> > classes.
>
> Yes, this causes confusion quite often.
>
> I wanted to remove RefPtr::clear(), or at least deprecated it. I seem to
> have had some problem creating an operator=() to replace it:
>
> From
> http://cvs.gnome.org/viewcvs/glibmm/ChangeLog?view=markup
> "
> Wanted to remove clear()
> 	too, but there is no =0 equivalent yet.
> "
>
> It's a bit late now (we can't break API), but a patch would be welcome.

At the moment Glib::RefPtr follows the example of many reference counting 
smart pointers by only taking a pointer to the managed type in its 
constructor - the intention being that any assignment should be from another 
RefPtr so the reference count can be incremented.  Where a pointer assignment 
is allowed, then it must reseat the referenced object and start a new 
reference count for the reseated object of 1.

If you want to allow assignment of a NULL pointer to a Glib::RefPtr you would 
need to implement such reseating, and also handle the special case of a NULL 
pointer to vacate the existing reference and put the RefPtr into a null 
state.  That would change the API and does not seem to be an approach 
favoured by many smart pointers.

std::auto_ptr and boost::shared_ptr have a reset() method but I am not sure if 
that is better than a clear() method.  boost::shared_ptr does not allow 
assignment of a pointer by operator=(), but you can reseat the referenced 
object using an overloaded reset() method which takes a pointer.  Requiring a 
call to Glib::RefPtr::reset(0) to reset the RefPtr would make it more 
difficult to do the wrong thing by mistake, and could be implemented as part 
of a new reseating method.

Chris