check out my smart pointer class

Started by
12 comments, last by Zahlman 15 years, 4 months ago
Quote:Original post by me22
First bug I saw: You leak memory if the allocation of the count fails.

Moral of the story: Smart pointers are hard to write. Use somebody else's -- especially now that GCC and MSVS both provide std::tr1::shared_ptr.


I am trying to use std::tr1::shared_ptr and have included <memory> but VS gives the following error:

error C3083: 'tr1': the symbol to the left of a '::' must be a type,

when making the following statement:

typedef std::tr1::shared_ptr<SomeType> SomeTypePtr;

I looked at memory and the files it includes and there is no declaration of tr1 or shared_ptr. Is the VS implementation for tr1 an add-on I need to install?

Thanks.
Advertisement
Quote:Original post by impulsionaudio
Quote:Original post by me22
First bug I saw: You leak memory if the allocation of the count fails.

Moral of the story: Smart pointers are hard to write. Use somebody else's -- especially now that GCC and MSVS both provide std::tr1::shared_ptr.


I am trying to use std::tr1::shared_ptr and have included <memory> but VS gives the following error:

error C3083: 'tr1': the symbol to the left of a '::' must be a type,

when making the following statement:

typedef std::tr1::shared_ptr<SomeType> SomeTypePtr;

I looked at memory and the files it includes and there is no declaration of tr1 or shared_ptr. Is the VS implementation for tr1 an add-on I need to install?

Thanks.


You will need Visual Studio 2008 and the latest service pack. I don't believe there is a SP or addition for 2005 or previous.
You may delete #pragma once since it is compiler dependent and you already have ifdef guards.

SmartPtr::GetCount should not exist since it is an implementation detail not directly related to the SmartPtr (or at least call it GetCountReferencesToInternalObject, GetCount alone is not very descriptive).

You should implement operator* which returns *m_pInstCounted and get() or GetPtr() which returns m_pInstCounted, and maybe also release() and reset() (see boost::shared_ptr).

About constness:
operator== and operator!= (and the famelic GetCOunt) should be const member functions.

Expanding frob thoughts, implementing "T* SmartPtr<T*>::operator->() const" you would break the rule "const should not expose non const", but since the assignment operator and the copy constructor already break it, there is no sense trying to preserve constness. Here how to break constness with your implementation:
void func(const SmartPtr<T*>& p){  //p->NonConst(); // forbidden!  SmartPtr<T*> q(p);  q->NonConst(); // allowed!}

To preserve constness you have to implement assignment operators and copy constructors in a different way. Something like:

SmartPtr<T*>::operator=(SmartPtr<T*>& other);SmartPtr<const T*>::operator=(const SmartPtr<T*>& other);SmartPtr<const T*>::operator=(const SmartPtr<T*>& other);

Or

SmartPtr<T*>::operator=(SmartPtr<T*>& other);ConstSmartPtr<T*>::operator=(const SmartPtr<T*>& other);ConstSmartPtr<T*>::operator=(const ConstSmartPtr<T*>& other);

But that's so hard to implement, use and maintain that java and c# do not even have the c++ concept of constness.
Quote:Original post by Nyarlath
You may delete #pragma once since it is compiler dependent and you already have ifdef guards.


May; but it doesn't really hurt, and it might help the "dependent" compilers build the project faster. (This isn't a reason to spontaneously start using them in a new project, though. YAGNI, etc.)
Quote:Original post by Rydinare
Sorry for stating the obvious, but why not use boost::shared_ptr<>?
One thing to note in comparison to shared_ptr is that above implementation does not have the same thread safety guarantees that shared_ptr has.
Now, think about it as you may. In my opinion, that is even an advantage, because it's functionality that I've never used and that's associated with overhead.
However, if it happens that you do need those guarantees, you'll spend weeks trying to figure out why your program crashes.

This topic is closed to new replies.

Advertisement