If you're using Boost, you could use the intrusive_ptr for this purpose.Originally Posted by CornedBee
If you're using Boost, you could use the intrusive_ptr for this purpose.Originally Posted by CornedBee
Last edited by Sang-drax; 12-16-2004 at 08:09 AM.
Last edited by Sang-drax : Tomorrow at 02:21 AM. Reason: Time travelling
Indeed, I've wondered all the time.
All the buzzt!
CornedBee
"There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
- Flon's Law
>>Then you might be able to create a default behavior where a delete functor is called.
Different implementations of this have already been suggested by Zach and myself. I know mine works, though my compiler doesn't seem to handle Zach's version properly (unless it really isn't supposed to work).
And I really don't see why CornedBee's check/set NULL solution for the destructor problem wouldn't work?
**EDIT**
Huh? Wha? There IS only one reference count for each object, isn't there?the problem is that you have several reference counts floating around for the same object, making it way too easy to destroy the object at the wrong time - you should have one and only one counter for each object.
Just Google It. √
(\ /)
( . .)
c(")(") This is bunny. Copy and paste bunny into your signature to help him gain world domination.
His code looks okay. Make sure you have the space between the two > brackets. Another way might be to writeDifferent implementations of this have already been suggested by Zach and myself. I know mine works, though my compiler doesn't seem to handle Zach's version properly (unless it really isn't supposed to work).
Code:template<typename T> void generic_delete(T& ptr) { delete ptr; ptr = 0; } template<typename T, void (*GD)(T& ptr) = generic_delete<T> > class refptr { //...// };If it's just a matter of someone purposely calling the destructor, I think then it's reasonable to release the resources. In fact, it's most likely what that person wanted. But for those who actually need to call the destructor without the reference decrementing, they could first increase the reference count and then call the destructor. Some type of addRef operation would have to be public, though.And I really don't see why CornedBee's check/set NULL solution for the destructor problem wouldn't work?
>> I might be missing something, but it appears that the reference count will not properly be updated if you reassign the smart pointer.
>> If you do an assignment, the reference count of the original pointer never gets decremented (and accordingly, the pointer doesn't get freed
>> That's what I just pointed out.
>> You should be able to pass the onZeroRef function as a template...
I drove all the way home from work thinking, "something's not right.....well, they'll find it". The sad part is that I tried to model it after a known working implementation of mine but didn't look at operator=().
>> make the ZeroRef function a virtual member
I tend to use inheritance in a design as a last resort.
>> The user will be forced to use the explicit constructor even if you leave out this line.
>> Implement operator * and operator bool
<Cartman>"Sweeeeeeeeet".
A get() method seems fairly common too.
>> Also, perhaps the ZeroRef function itself would go well as a template parameter
Excellent idea [everyone]. I shy'd away from it at first since it changes the type of the smart pointer type. But on second thought, I like that since you wouldn't want to assign two smart pointers with different ZeroRef functors. Plus, it's more inline with the std:: way of doing it.
>> template<class T, class ZeroRefT = generic_delete<T> >
That would probably be the most likely usage scenario. I like it.
>> You could also create an internal templated ZeroRef function (protected, static), use that as the default [template] argument.
Yes, but the more std:: thing to do is to have a standalone unary_function<T, void> derivative.
>> the problem is that you have several reference counts floating around for the same object, making it way too easy to destroy the object at the wrong time
>> Huh? Wha? There IS only one ... isn't there?
I don't see the multiple reference counts either. Shall we chalk it up to smoke'n or drink'n or both?
>> the user could possibly mess up the count by calling ...destructors manually
I have to file that under: "if it hurts, don't do it". However, it should be mentioned in a proper tutorial It could also be misused by calling the "ZeroRef" functor on a pointer manually. CornedBee and Sebastiani have nicely covered what not to do.
>> I'd use a Boost.Function object to store the deleter.
This seems like overkill to me since the fuctor's signature will always return void and take a pointer to T. If you need a different signature to be called, then using an adapter class is the more std:: way. This is also how boost::shared_ptr handles it.
Of course, for a truly robust implementation, use boost::shared_ptr for better:
- exception safety
- thread safety
- handling of different types of T
- debug'n facilities...and the list goes on...
Having said that, here are the changes for the new and improved version:
- used a more std:: naming style
- removed "using namespace std" to simulate the class being in a proper header (should be within a namespace as well)
- added operator*, operator bool, and T* get()
- added ref_count() member (rip off from boost::shared_ptr)
- made access methods (and operator bool) const
- made reference counting (more) correct
- "ZeroRef" functor is now a defaulted templated parameter
We should do this more ofterCode:#include <functional> template<class T> struct delete_ptr : public std::unary_function<T, void> { void operator()(T *&ptr) const {delete ptr; ptr = 0;} };//delete_ptr template <class T, class F = delete_ptr<T> > class ref_ptr { protected: F m_unload; size_t *m_refs; T *m_pointer; // copy 'rcp' into '*this' and increment reference count ref_ptr& copy(const ref_ptr &rcp) { if (m_refs && (m_refs == rcp.m_refs)) return *this; m_refs = rcp.m_refs; m_pointer = rcp.m_pointer; (*m_refs)++; return *this; }//copy // decrement reference count and call m_unload on 0 void dec_ref() { if (--(*m_refs) == 0) { delete m_refs; m_refs = 0; m_unload(m_pointer); }//if }//dec_ref public: // construction / destruction explicit ref_ptr(T *p, const F &f = F()) : m_pointer(p), m_unload(f), m_refs(new size_t(1)) {} ~ref_ptr() {dec_ref();} // copy semantics ref_ptr(const ref_ptr &rcp) {copy(rcp);} ref_ptr& operator=(const ref_ptr &rcp) {dec_ref(); return copy(rcp);} // access T& operator*() const {return *m_pointer;} T* operator->() const {return m_pointer;} T* get() const {return m_pointer;} // conditional operator bool() const {return m_pointer != 0;} size_t ref_count() const {return m_refs ? *m_refs : 0;} };//ref_ptr //----------------------------------------------------------------------------- #include <iostream> #include <vector> using namespace std; struct A { A() {cout << "constructor" << endl;} ~A() {cout << "destructor" << endl;} };//A void DeleteA(A *p) { cout << "DeleteA - "; delete p; }//DeleteA int main() { // use default "unload" functor ref_ptr<A> rcp1(new A); // use DeleteA() as the "unload" functor ref_ptr<A, void(*)(A*)> rcp2(new A, &DeleteA); vector<ref_ptr<A> > v1; v1.push_back(rcp1); v1.push_back(rcp1); v1.push_back(rcp1); v1.push_back(rcp1); cout << "Ref Count = " << rcp1.ref_count() << endl; vector<ref_ptr<A> > v2 = v1; v2.erase(v2.begin()); v2.erase(v2.begin()); v2[0] = v2[1]; // <- brand new test case!!! // should only be 2 constructions and destructions return 0; }//maim
gg
Last edited by Codeplug; 12-16-2004 at 05:07 PM.
Well, it looks good.
If I must say something, I'd say make ref_count() const as well.
Last edited by Sang-drax : Tomorrow at 02:21 AM. Reason: Time travelling
Why not? Just assign the deleter along with the pointer. Since the old pointer either gets deleted or released to the care of other smart pointers, you don't need the old deleter anymore. So you can just take a copy of the new one. (That's why Boost's shared_ptr stores the deleter in the same struct as the share count, btw.)Originally Posted by Codeplug
Boost.Function isn't about different signatures (it's templated on the signature), but about different callable objects, such as function pointers, functors, ...>> I'd use a Boost.Function object to store the deleter.
This seems like overkill to me since the fuctor's signature will always return void and take a pointer to T. If you need a different signature to be called, then using an adapter class is the more std:: way. This is also how boost::shared_ptr handles it.
The advantage would be that you could assign both of these as deleters:
Code:template<typename T> void my_del_function(T *& p) { delete p; p = 0; }For getting both of these, you either have to template the smart pointer on the deleter type (as I explained above, I don't think that makes much sense) or use something akin to Boost.Function.Code:template<typename T> struct my_del_functor { void operator()(T *& p) { delete p; p = 0; } };
All the buzzt!
CornedBee
"There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
- Flon's Law
>> I don't see the multiple reference counts either. Shall we chalk it up to smoke'n or drink'n or both?
nah, just me head up me arse.
Code:#include <cmath> #include <complex> bool euler_flip(bool value) { return std::pow ( std::complex<float>(std::exp(1.0)), std::complex<float>(0, 1) * std::complex<float>(std::atan(1.0) *(1 << (value + 2))) ).real() < 0; }
Also, perhaps you could add a second 'built-in' delete functor, delete_array And that's where it becomes nice if you use the function pointer method: You can reassign a pointer-to-object to a pointer-to-array and vice versa, as long as the type you're pointing to is the same. I also wrote a [] operator, and scattered liberal asserts around to bugger the hell out of anyone who manual-destructs the thing several times and then tries copying it into another smartpointer.
The changes to dec_ref just ensure that even if the destructor is called manually, the reference count will only be decremented once.Code:void dec_ref() { if(m_refs == 0) return; if (--(*m_refs) == 0) { delete m_refs; m_refs = 0; m_unload(m_pointer); }//if m_refs = 0; }//dec_ref
If the functor type is already locked in stone, why do you need to pass an object of it as an argument to the constructor? The functor specified in the template should always be the same as the one given to the constructor anyway.Code:explicit ref_ptr(T *p, const F &f = F())
Why do you need that? If both m_refs and rcp.m_refs are 0, then you won't end up returning at this point, and you'll dereference a NULL pointer.Code:if (m_refs && (m_refs == rcp.m_refs))
**EDIT** Oops, you'll end up dereferencing it anyway as long as rcp.m_refs == 0 and m_refs isn't. That's one of the spots that I assert()'ed myself on But the other thing is, it shouldn't matter if m_refs is NULL or not before you copy another ref_ptr; if it's NULL, then del_ref() will just do nothing (the pointer's already been taken care of, somehow), and then you'll make a copy of rcp, incrementing rcp's reference count, and life goes on.
Last edited by Hunter2; 12-16-2004 at 05:04 PM.
Just Google It. √
(\ /)
( . .)
c(")(") This is bunny. Copy and paste bunny into your signature to help him gain world domination.
Hunter:
delete m_refs;
m_refs = 0;
The second line is redundant, as it is executed unconditionally after the if anyway.
All the buzzt!
CornedBee
"There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
- Flon's Law
Wups. Actually, it was Codeplug's line, I just forgot to delete it when I moved it out of the if block
**P.S.
What do all the const's do? I thought they just mean that the function won't be modifying member variables, and therefore allows them to be called on const objects of the class - but since operator-> etc. return T*, that allows them to modify the contents of the T pointed to, right? But isn't it a bad thing if a const ref_ptr is allowed to modify whatever it's pointing to? So perhaps it would be better to create two definitions of each operator:
T* operator->()
const T* operator->() const
T* get()
const T* get() const
etc.
Last edited by Hunter2; 12-16-2004 at 05:25 PM.
Just Google It. √
(\ /)
( . .)
c(")(") This is bunny. Copy and paste bunny into your signature to help him gain world domination.
This will break on self-assignment, won't it?Code:ref_ptr& operator=(const ref_ptr &rcp) {dec_ref(); return copy(rcp);}
Worse, it will break sometimes. It will work if dec_ref doesn't have safety precautions (such as setting to null) and there's still another smart pointer referring to the object.
Admittedly, the current version does have these safety precautions, so they will always break
Not at all. ABut isn't it a bad thing if a const ref_ptr is allowed to modify whatever it's pointing to?
const ref_ptr<int>
is equivalent to a
int *const
.
Works. To have a pointer-to-const, pass a const type as template parameter.Code:int i = 0; int *const p = &i; *p = 5; std::cout << i << std::endl;
ref_ptr<const int>
Last edited by CornedBee; 12-16-2004 at 05:50 PM.
All the buzzt!
CornedBee
"There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
- Flon's Law
Hmm... is it really a good idea to disallow storage of null? Neither the std nor the boost smart pointers do it. Sometimes a null pointer can be really useful.
Edit: Uh, did someone just delete a post before this one?
All the buzzt!
CornedBee
"There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
- Flon's Law
Yup. I meant to get rid of all my overloaded const operators and repost it, but I'm going for a walk now
**Pre-walk edit:
Sure it might not be a bad thing, but it'll take a little extra effort to handle that case. Otherwise you'll have reference counting for NULL
Just Google It. √
(\ /)
( . .)
c(")(") This is bunny. Copy and paste bunny into your signature to help him gain world domination.