Asfand Yar Qazi wrote:[color=blue]
> Hello. Partly for learning purposes, I have written a smart pointer class.
>
> Could you please tell me what's wrong with it? (I know there's
> something wrong with it, but just not what!)
>
> Note that, as well as reference counting the stored pointer, it also has
> the ability to be declared for incomplete types (I think). I.e.:
>
> struct Incomplete;
>
> Klever_Ptr<Incomplete> p;
>
> ..
>
> struct Incomplete
> {
> };
>
> p.reset(new Incomplete);[/color]
boost::shared_ptr does this.
[color=blue]
>
>
>
> ======= BEGIN CODE =======
>
> #include <typeinfo>
>
> /// Klever pointer class - the stored pointer will be deleted when the
> /// last Klever_Ptr pointing to it is deleted!
> template<class T>
> class Klever_Ptr
> {
> private:
> class Impl_Base
> {
> protected:
> friend class Klever_Ptr;
>
> Impl_Base(T* argptr) throw()
> : ptr(argptr), refcount(1)
> {
> }
>
> /// Child classes only delete the pointer, we set it
> /// to 0
> virtual
> ~Impl_Base() throw() = 0;
>
> T* ptr;
>
> long refcount;
>[/color]
I would add methods to increment and decrement refcount here.
[color=blue]
> }; // class Impl_Base
>
> template<class U>
> class Impl : protected Impl_Base
> {
> friend class Klever_Ptr;
>
> /// U must be complete, and a child of/same as T
> Impl(U* argptr) throw()
> : Impl_Base(argptr)
> {
> typedef char type_must_be_complete[sizeof(U)];
> }
>
> ~Impl() throw()
> {
> delete this->ptr;
> }
>
> }; // class Impl
>
> Impl_Base* pimpl;
>
> public:
> /// Default constructor - leaves the Klever_Ptr open to
> /// becoming a co-owner of another Klever_Ptr, but accessing
> /// the value before assigning a new value is undefined!
> Klever_Ptr()
> : pimpl(0)
> {
> }
>
> /// Initialise this Klever_Ptr with a return value of
> /// 'operator new' - i.e. it will be deleted when the last of
> /// the Klever_Ptrs pointing to it are deleted!
> template<class V>
> explicit
> Klever_Ptr(V* argptr)
> : pimpl(new Impl<V>(argptr))
> {
> }
>[/color]
What do you think you code does with this:
class A{ virtual ~A(); };
class B : public A {};
Klever_Ptr<B> pb = new B;
Klever_Ptr<A> p = pb;
[color=blue]
> /// Create a new owner of the given pointer
> Klever_Ptr(const Klever_Ptr& arg) throw()
> : pimpl(arg.pimpl)
> {
> if(this->pimpl)
> ++this->pimpl->refcount;
> }
>
> /// Disown the current stored pointer, and become a new owner
> /// of the given one.
> Klever_Ptr&
> operator=(const Klever_Ptr& arg) throw()
> {
> if(this == &arg)
> return *this;[/color]
VVVVVVVVVVVVVVVVVVVVVV[color=blue]
> this->disown();
> this->pimpl = arg.pimpl;
> if(this->pimpl) // if arg is NULL pointer...
> ++this->pimpl->refcount;
> return *this;[/color]
^^^^^^^^^^^^^^^^^^^^^^
Make sure you increment and place the new pointer in place
before you disown the old pointer. Major bugs happen right
here when breaking down complex structures.
[color=blue]
> }
>
> /// Disown the stored pointer, deleting it if necessary
> ~Klever_Ptr() throw()
> {
> this->disown();
> }
>
> /// Access the value (I find operator* useless.)
> T*
> ptr() const throw()
> {
> return this->pimpl->ptr;
> }[/color]
2 dereferences ... mmm
[color=blue]
>
> /// Access the value as a different type using dynamic_cast -
> /// throws std::bad_cast on invalid cast
> template<class V>
> V*
> ptr() const throw()
> {
> T* tmp = dynamic_cast<V>(this->pimpl->ptr);
> if(tmp == 0)
> throw std::bad_cast();
> return tmp;
> }
>
> /// Return a reference
> T&
> ref() const throw()
> {
> return *this->ptr();
> }
>
> /// Indirect-call the pointer
> T*
> operator->() const throw()
> {
> return this->ptr();
> }
>
> /// Disown the current pointer, and assign a new one - the
> /// argument supplied must be the result of an 'operator new'
> /// call - @c Klever_Ptr(T*)
> template<class V>
> void
> reset(V* arg)
> {
> this->disown();
> this->pimpl = new Impl<V>(arg);
> }[/color]
What about the case for arg being null ?
[color=blue]
>
> /// Remove yourself as an owner of the stored pointer - if you
> /// were the last one, delete it - accessing the pointer from
> /// now on is undefined!
> void
> disown()
> {
> if(this->pimpl == 0)
> return;
> --this->pimpl->refcount;
> if(this->pimpl->refcount == 0)
> delete this->pimpl;
> this->pimpl = 0;
> }
>
> /// Returns the number of owners
> long
> get_ref()
> {
> if(this->pimpl)
> return this->pimpl->refcount;
> else
> return 0L;
> }[/color]
Why would you ever need a reference count ?
[color=blue]
>
> }; // class Klever_Ptr
>
> template<class T>
> Klever_Ptr<T>::Impl_Base::~Impl_Base() throw()
> {
> // child classes responsibility to delete the pointer
> ptr = 0;
> }[/color]
