##Common biggest mistake.
Common biggest mistake.
##Common biggest mistake.
Common biggest mistake.
Optimization
You create an object to hold the count.
In the shared pointer you keep a pointer to this count object and a destructor object. When you copy/move both these values need to be updated. Why not modify the count object to store all the accessory information about the shared pointer (both count and destructor object). That way when you copy/move you just need to move one pointer (in addition to the data).
Missing Functionality
When you have de-reference and de-refrence method functions you should really have a way to check that the object can be de-referenced. Otherwise you have to hope it does not blow up when you use these methods.
Basically you need some way to check the pointer you are holding is not a nullptr.
explicit operator bool() const {return managed;}
Inconsistent behavior
If you explicitly construct a shared pointer with a nullptr and you create a shared pointer using the default constructor (this internally it is nullptr) you have different internal structures (one has a user count the other does not).
Shrd_ptr<int> data1(nullptr);
Shrd_ptr<int> data2;
I can not see any bugs with this. But as your class gets bigger having this inconsistency may cause unforeseen issues. No matter how the object is constructed it should have the same internal structure for the same data.
No nullptr constructor.
You don't have a constructor for explicitly taking nullptr as a parameter.
But you say nullptr binds to any pointer type. Yes. But its not about the normal situation.
This is a situation where you are passing a nullptr to function/method. If the function takes an r-value ref we can not construct the object (as the constructor is explicit) so you need to fully qualify the nullptr to make the call.
void workWithSP(Shrd_ptr<int>&& sp); // Some definition.
int main()
{
workWithSP(nullptr); // Does not work.
workWithSP(Shrd_ptr<int>(nullptr)); // Works but seems verbose.
}
Simply add a nullptr_t constructor.
Shrd_ptr::Shrd_ptr(std::nullptr_t): ....
Missing Functionality
Assigning derived types.
class X {};
class Y: public X {};
Shrd_ptr<Y> data(new Y);
Shrd_ptr<X> dataNew(std::move(data)); // Why does that not work?
Optimization
You create an object to hold the count.
In the shared pointer you keep a pointer to this count object and a destructor object. When you copy/move both these values need to be updated. Why not modify the count object to store all the accessory information about the shared pointer (both count and destructor object). That way when you copy/move you just need to move one pointer (in addition to the data).
Missing Functionality
When you have de-reference and de-refrence method functions you should really have a way to check that the object can be de-referenced. Otherwise you have to hope it does not blow up when you use these methods.
Basically you need some way to check the pointer you are holding is not a nullptr.
explicit operator bool() const {return managed;}
Inconsistent behavior
If you explicitly construct a shared pointer with a nullptr and you create a shared pointer using the default constructor (this internally it is nullptr) you have different internal structures (one has a user count the other does not).
Shrd_ptr<int> data1(nullptr);
Shrd_ptr<int> data2;
I can not see any bugs with this. But as your class gets bigger having this inconsistency may cause unforeseen issues. No matter how the object is constructed it should have the same internal structure for the same data.
No nullptr constructor.
You don't have a constructor for explicitly taking nullptr as a parameter.
But you say nullptr binds to any pointer type. Yes. But its not about the normal situation.
This is a situation where you are passing a nullptr to function/method. If the function takes an r-value ref we can not construct the object (as the constructor is explicit) so you need to fully qualify the nullptr to make the call.
void workWithSP(Shrd_ptr<int>&& sp); // Some definition.
int main()
{
workWithSP(nullptr); // Does not work.
workWithSP(Shrd_ptr<int>(nullptr)); // Works but seems verbose.
}
Simply add a nullptr_t constructor.
Shrd_ptr::Shrd_ptr(std::nullptr_t): ....
Missing Functionality
Assigning derived types.
class X {};
class Y: public X {};
Shrd_ptr<Y> data(new Y);
Shrd_ptr<X> dataNew(std::move(data)); // Why does that not work?
##Common biggest mistake.
The largest common problem is not ensuring ownership is taken during construction. The problem is that if the constructor does not complete (ie an exception is thrown out of the constructor) then the destructor will never be called.
But so what you ask. Well if you blow up during construction you have definitely leaked the pointer you just promised to manage!
explicit Shrd_ptr(T* managed_, destructor_type destructor_ = nullptr)
: Smart_pointer_base<T>(managed_)
, user_count(new size_type(1))
, destructor(std::move(destructor_))
{ }
It looks so simple. How can that not complete? Well the you have a call to new in there. It is not exception safe and can throw. So what can you do about it?
Two options. 1) Use a non throwing new: new (std::nothrow) size_type(1) 2) catch exceptions in the constructor using Function try blocks.
# Option 1
explicit Shrd_ptr(T* managed_, destructor_type destructor_ = nullptr)
: Smart_pointer_base<T>(managed_)
, user_count(new (std::nothrow) size_type(1))
, destructor(std::move(destructor_))
{
if (user_count == nullptr) {
destructor(managed_);
throw std::bad_alloc("Failed");
}
}
# Option 2
explicit Shrd_ptr(T* managed_, destructor_type destructor_ = nullptr)
try
: Smart_pointer_base<T>(managed_)
, user_count(new size_type(1))
, destructor(std::move(destructor_))
{}
catch(...)
{
destructor_(managed_);
throw; // re-throw current exception
}
Virtual Functions
Calling virtual functions in the constructor or destructor is UB. Because it is implementation defined how virtual functions are implemented the standard has put restrictions on when they can be used. The can not be used during construction or destruction.
Shrd_ptr<T>::~Shrd_ptr()
{
destruct(); // This is UB
}
Since each shared pointer is unique. Just put the code you have in destruct() in the destructor for that type of object. That will do exactly what you expect:
Shrd_ptr<T>::~Shrd_ptr()
{
if (safe_decrement(user_count) == 0) {
delete user_count;
destructor ? destructor(this->managed) : Smart_pointer_base<T>::destruct();
}
}
Unq_ptr<T, destructor_type>::~Unq_ptr()
{
destructor(this->managed);
}