How to delete raw pointers from a unique_ptr

Question

I'm writing a game framework, I have a vector<unique_ptr<Object>> list and I distribute pointers from that list by calling object.get() and sending that out. Before that I send out references instead of raw pointers but that resulted in other weird problem so I was told this is better. However when I remove a unique_ptr<Object>from the list, the raw pointers remains. I also can't deallocate them manually, I get an exception saying the pointer is not allocated.

So my question would be:

1. How do I delete raw pointers from removed unique_ptr's?

and is also a more general question:

2. Am I on the right track structure wise of passing pointers instead of references?

PxlObject* PxlFramework::AddObject(PxlObject* obj)
{
    std::unique_ptr<PxlObject> u_ptr(obj);
    objects_iterator = objects.insert(objects.end(), std::move(u_ptr));
    return obj;
}

void PxlFramework::DeleteObject(PxlObject* obj) {
    for(objects_iterator = objects.begin(); objects_iterator != objects.end(); ++objects_iterator)
    {
        if((*objects_iterator)->get_id() == obj->get_id())
        {
            //attempting to delete the raw pointer here but it will result in an error
            delete obj;

            //removing the unique_ptr<PxlObject> from the list here
            std::swap((*objects_iterator), objects.back());
            objects.pop_back();
            break;
        }
    }
}

Answer 1

You don't need delete the raw pointer directly you can use vector.erase instead. Here you have a simple example:

#include <iostream>
#include <algorithm>
#include <memory>
#include <vector>

using namespace std;

typedef vector<unique_ptr<int>> ptr_list_t;

void remove_number(int x, ptr_list_t& ptr_list)
{
    for (ptr_list_t::iterator it = ptr_list.begin(); it != ptr_list.end(); ++it)
        if (*(it->get()) == x) {
            ptr_list.erase(it);     // Use vector.erase for deleting objects from a vector.
                                    // since it points to a unique_ptr, the object owned by it
                                    // will be destroyed automatically.
            break;
        }
}

int main()
{
    ptr_list_t ptr_list;

    // Generating the pointer to numbers. 0 - 9
    for (int i = 0; i < 10; i++)
        ptr_list.push_back(unique_ptr<int>(new int(i)));

    // Remove the number 3.
    remove_number(3, ptr_list);

    // Printing the list. The number 3 will not appear.
    for (ptr_list_t::iterator it = ptr_list.begin(); it != ptr_list.end(); ++it)
        cout << *(it->get()) << endl;

    return 0;
}

Other thing, I'm agreed with @MooingDuck: you should not delete either a unique_ptr nor anything that a unique_ptr owns. But you sure can. Take a look on unique_ptr.release. This function frees the ownership of the managed object.

Answer 2

The whole point of std::unique_ptr is that it "owns" the object and it manages deletion automatically when the unique_ptr is destroyed. As such, you should not delete either a unique_ptr nor anything that a unique_ptr owns. To avoid this confusion, references are more common. Additionally, you have the oddity that your AddObject returns a pointer to a PxlObject that is not the one just added.

Something like this might be a little cleaner:

template<class Us...>
PxlObject& PxlFramework::AddObject(Us&&... obj)
{
    std::unique_ptr<PxlObject> u_ptr(new PxlObject(std::forward<Us>(obj)...));
    objects_iterator = objects.insert(objects.end(), std::move(u_ptr));
    return **objects_iterator;
}

void PxlFramework::DeleteObject(PxlObject& obj) {
    auto finder = [](std::unique_ptr<PxlObject>& p)->bool 
            {return obj.get_id()==p->get_id();};
    auto it = find_if(objects.begin(), objects,end(), finder);
    if (it != objects.end())
        objects.erase(it);
    else
        throw ...;
}