From: Warren Young
Date: November 29 2007 8:05am
Subject: Re: Eyeballs needed on new reference counted pointer template
List-Archive: http://lists.mysql.com/plusplus/7197
Message-Id: <474E72B9.402@etr-usa.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Jonathan Wakely wrote:
> Hi Warren, here's a modified RefCountedPointer, with the following changes:

Mostly applied as-is, with some changes detailed below.

You've now made it into the CREDITS file.  :)

> * Added a no-throwing swap operation. 

Is there a good reason for this to be public?  Isn't it just an 
implementation detail for assign()?

> (Should probably define a global
> swap(x, y) to use this member)

Why?

> * Safe bool idiom replaces operator bool and operator!

I was going to do this, but just yesterday I found the need in 
RefCountedPointer for operator== and operator!= as well.  I can get that 
and also implicit conversion to bool-like with operator void*.  The safe 
bool article warns against it in part because it lets you say:

	RefCountedPointer<Foo> bar(new Foo);
	delete bar;

But *really*.  If one is so intent on shooting oneself in the foot....

The only time this should happen accidentally is when converting an 
unmanaged pointer to a refcounted pointer and forgetting to remove the 
deletes you previously needed.  Such a bug will show up the first time 
you run the program as it results in a double-delete.  I've documented 
the risk in the refman and am content to leave it at that.

The other warnings in the article about operator void* don't really 
apply here.  RefCountedPointer _is_ a pointer after all.  It's perfectly 
legitimate that you should be able to convert it to void*, unlike in the 
example he gives where cin/cout have no pointerness to them.

Your safe-bool code may still make its way into MySQL++, because there 
are other operator bools on classes that, like cin/cout also have no 
pointerness to them.

> ...safe against self-assignment...

I don't see any checks for self-assignment in your code.  It sort of 
happens implicitly in assign(const ThisType&) because the refcount just 
goes up and then comes right back down, but this is wasteful.  And in 
the assign(T*) case, I'm pretty sure assigning the same pointer will 
result in a double-delete.  I've added explicit checks to both assign() 
overloads.