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.
