List:MySQL++« Previous MessageNext Message »
From:Joseph Artsimovich Date:August 15 2007 8:57am
Subject:Re: Eyeballs needed on new reference counted pointer template
View as plain text  
There are several problems with your RefCountedPointer template:

1. It's not thread-safe.  You can't reference a single object from multiple 
threads.  To be able to do so, incrementing and decrementing the reference 
counter must be atomic.

2. It's not exception-safe, as was already pointed out.  In assign() you first 
call detach(), and then allocate memory.  Even if you would allocate memory 
before detach(), that would not solve the problem, as you would still leak 
the argument to assign() in case memory allocation fails.
The correct way to implement assign is:
void assign(T* c)
{
	ThisType(c).swap(*this);
}
Where swap() just swaps the pointers.  The same implementation can be used for 
assign(const ThisType& c)

3. It should be possible to do this:
class A {};
class B : public A {};
RefCountedPointer<B> pb(new B);
RefCountedPointer<A> pa(pb);
To achieve this, you need to create a template constructor and assignment 
operator, like this:
template<typename T>
template<typename OT>
RefCountedPointer<T>::RefCounterPointer(const RefCountedPointer<OT>&
other)
{
	// the usual stuff here
}

4. Implicit convertion to bool is dangerous.  It allows code like this to 
compile:
RefCountedPointer<A> pa;
int a = pa;
Even worse, because you haven't provided comparison operators, comparing two 
smart pointers will now involve converting both of them to bool!
Some books suggest providing an implicit conversion to a pointer-to-member 
instead of bool.  That's slightly better, but it doesn't solve all the 
problems.  My opinion is that you either don't provide implicit conversion at 
all, or you provide implicit conversion to T*, which is also dangerous, but 
in practice I never had troubles with this approach.

Regards,
	Joseph Artsimovich
Thread
Eyeballs needed on new reference counted pointer templateWarren Young15 Aug
  • Re: Eyeballs needed on new reference counted pointer templateChris Frey15 Aug
    • Re: Eyeballs needed on new reference counted pointer templateWarren Young16 Aug
  • Re: Eyeballs needed on new reference counted pointer templateAlex Burton15 Aug
    • Re: Eyeballs needed on new reference counted pointer templateWarren Young16 Aug
  • Re: Eyeballs needed on new reference counted pointer templateJoseph Artsimovich15 Aug
    • Re: Eyeballs needed on new reference counted pointer templateJonathan Wakely16 Aug
      • Re: Eyeballs needed on new reference counted pointer templateWarren Young16 Aug
        • Re: Eyeballs needed on new reference counted pointer templateJonathan Wakely18 Aug
          • Re: Eyeballs needed on new reference counted pointer templateJonathan Wakely18 Aug
          • Re: Eyeballs needed on new reference counted pointer templateWarren Young20 Aug
    • Re: Eyeballs needed on new reference counted pointer templateWarren Young16 Aug
  • RE: Eyeballs needed on new reference counted pointer templateJoel Fielder15 Aug
    • Re: Eyeballs needed on new reference counted pointer templateWarren Young16 Aug
  • Re: Eyeballs needed on new reference counted pointer templateRobert Mecklenburg15 Aug
    • Re: Eyeballs needed on new reference counted pointer templateGraham Reitz15 Aug
    • Re: Eyeballs needed on new reference counted pointer templateGraham Reitz15 Aug
    • Re: Eyeballs needed on new reference counted pointer templateWarren Young16 Aug
  • Re: Eyeballs needed on new reference counted pointer templateGraham Reitz15 Aug