Deleting an object via std::weak_ptr

Question

I have a std::vector with thousands of objects, stored as shared_ptr. Since the object has many attributes that can be used for searching, I maintain multiple indexes to this std::vector on std::map and std::multimap using weak_ptr.

std::vector<std::shared_ptr<Object>> Objects;
std::map<int,std::weak_ptr<Object>> IndexByEmployeeId;
std::multimap<std::string,std::weak_ptr<Object>> IndexByName;

Since the map and multimap are balanced binary trees, the search/modify are super fast. However, I am a bit foxed about delete. I want to delete after looking up the object via one of the indexes. Locking the weak_ptr gives me shared_ptr, but it doesn't allow me to destroy object on the vector. Is there any way to delete the original object on the vector?

Answer 1

It seems from what you've posted that your data structures are inappropriate for what you want to do.

1. shared_ptr should only be used to express shared ownership. However, your posted code and your desire to delete the objects pointed to indicate that Objects is in fact the sole owner of its objects.
2. Your vector<shared_ptr<object>> appears to be used only as data storage container (the desired functionality of searching by id or name is implemented elsewhere), but removing elements from a vector is typically expensive, so you may better use another type of container.

If there are no other shared_ptr to your objects, then your design is poor. In this case you shouldn't use any smart pointers but simply container<object> and maps into the that. For example something like this (not tested not even compiled):

struct data_t { std::string name; /* more data */ };
using objt_map = std::map<std::size_t,data_t>;
using object   = objt_map::value_type;    // pair<const size_t,data_t>
using obj_it   = objt_map::iterator;
using name_map = std::multimap<std::string, obj_it>;
objt_map Objects;
name_map NameMap;

std::forward_list<obj_it> find_by_name(std::string const&name) const
{
    auto range = NameMap.equal_range(name);
    std::forward_list<obj_it> result;
    for(auto it=range.first; it!=range.second; ++it)
        result.push_front(it->second);
    return result;
}

std::forward_list<obj_it> find_by_id(std::size_t id) const
{
    auto it = Objects.find(name);
    return {it == Objects.end()? 0:1, it};
}

void insert_object(std::size_t id, data_t const&data)
{
    auto it = Objects.find(id);
    if(it != Objects.end())
        throw std::runtime_error("id '"+std::to_string(id)+"' already known");
    Objects[id] = data;
    NameMap.emplace(data.name, Objects.find(id));
}

void delete_object(obj_it it)
{
    if(it==Objects.end())
        throw std::runtime_error("attempt to delete invalid object");
    auto range = NameMap.equal_range(it->second.name);
    for(auto i=range.first; i!=range.second; ++i)
        if(i->second==it) {
            NameMap.erase(i);
            break;
        }
    Objects.erase(it);
}

Note iterators of std::map remain valid upon insertion and deletion (of other objects), such that the returns from the finders are not invalidated by insertion and deletion. I use std::forward_list<obj_it> to return objects to allow for returning none or several.