Search code examples
c++vectorunordered-set

Finding an index of element in vector and removing it


I have a loop. If a certain condition is true, I need to add an item to a vector. If it's false instead, I need to remove an item from the vector.

Here is my best attempt at reproducing what I am trying to do:

#include <iterator>
#include <vector>
#include <algorithm>
#include <memory>

struct Arrow
{
    std::vector<Arrow*> inRange;
};

int main()
{
    std::vector<std::unique_ptr<Arrow>> arrows;
    for (int i = 0; i < 10; i++)
        arrows.push_back(std::make_unique<Arrow>());

    for (int i = 0; i < arrows.size(); i++)
        for (int j = 0; j < arrows.size(); j++)
            if (i != j)
            {
                bool someCondition = true;//obviously can be false as well in the actual code
                if (someCondition)
                    arrows[i]->inRange.push_back(arrows[j].get());
                else
                {
                    std::vector<Arrow*> itr = std::find(arrows[i]->inRange.begin(),
                        arrows[i]->inRange.end(), arrows[i]);

                    int index = std::distance(arrows[i]->inRange, itr);
                    arrows[i]->inRange.erase(itr.begin(), index);
                }
            }
}

I am completely unfamiliar with all of this and have no idea what is going wrong. Thank you for helping.

Errors:

Severity    Code    Description Project File    Line    Suppression State
Error (active)  E0312   no suitable user-defined conversion from "std::_Vector_iterator<std::_Vector_val<std::conditional_t<true, std::_Simple_types<Arrow *>, std::_Vec_iter_types<Arrow *, size_t, ptrdiff_t, Arrow **, Arrow *const *, Arrow *&, Arrow *const &>>>>" to "std::vector<Arrow *, std::allocator<Arrow *>>" exists   steer away  C:\Users\me\source\repos\steer away\steer away\main.cpp 26  
Severity    Code    Description Project File    Line    Suppression State
Error (active)  E0304   no instance of overloaded function "std::vector<_Ty, _Alloc>::erase [with _Ty=Arrow *, _Alloc=std::allocator<Arrow *>]" matches the argument list   steer away  C:\Users\me\source\repos\steer away\steer away\main.cpp 30  
Severity    Code    Description Project File    Line    Suppression State
Error (active)  E0304   no instance of function template "std::distance" matches the argument list  steer away  C:\Users\me\source\repos\steer away\steer away\main.cpp 29  
Severity    Code    Description Project File    Line    Suppression State
Error (active)  E0304   no instance of overloaded function "std::vector<_Ty, _Alloc>::erase [with _Ty=Arrow *, _Alloc=std::allocator<Arrow *>]" matches the argument list   steer away  C:\Users\me\source\repos\steer away\steer away\main.cpp 30  
Severity    Code    Description Project File    Line    Suppression State
Error (active)  E0304   no instance of function template "std::distance" matches the argument list  steer away  C:\Users\me\source\repos\steer away\steer away\main.cpp 29  
Severity    Code    Description Project File    Line    Suppression State
Error   C2893   Failed to specialize function template 'iterator_traits<_Iter>::difference_type std::distance(_InIt,_InIt)' steer away  C:\Users\me\source\repos\steer away\steer away\main.cpp 29  
Severity    Code    Description Project File    Line    Suppression State
Error   C2664   'std::_Vector_iterator<std::_Vector_val<std::_Simple_types<_Ty>>> std::vector<_Ty,std::allocator<_Ty>>::erase(std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>,std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>) noexcept(<expr>)': cannot convert argument 2 from 'int' to 'std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<_Ty>>>'    steer away  C:\Users\me\source\repos\steer away\steer away\main.cpp 30  

Solution

  • I second the choice of vector, but since adding and removing items in a loop can be tricky and not so efficient I'd skip the adding and removing thing by starting from an empty container and then just adding, so this doesn't address directly your question but shows how I'd approach your problem. (code not compiled, may contain errors):

    
    class Arrow
    {
        /* ... */
        void clear_inrange_arrows(const std::size_t n)
        {
            m_inrange.clear();
            // This ensures just one dynamic allocation
            if(m_inrange.capacity()<n) m_inrange.reserve(n);
        }
        bool is_inrange_of(const Arrow& const other) const noexcept { /* ... */ }
        void add_inrange_arrow(const Arrow& const other)
        {
            m_inrange.push_back(&other);
        }
    
     private:
        std::vector<const Arrow*> m_inrange; // Arrows in range, owned by 'World'
    };
    
    // Allow me introduce something that manages
    // the memory and performs the operation
    class World
    {
        /* ... */
        std::vector<Arrow> all_the_arrows;
    
        /* ... */
        void precalculate_whos_inrange()
        {
        // For each existing arrow (you could use `auto` here)...
        for(std::vector<Arrow>::iterator icurr=all_the_arrows.begin(); icurr!=all_the_arrows.end(); ++icurr)
           {
            // ...Clear previous 'in-range' arrows container, and then...
            icurr->clear_inrange_arrows(all_the_arrows.size());
            // ...Add the other arrows in range
            for(auto iother=all_the_arrows.begin(); iother!=icurr; ++iother)
                if( icurr->is_inrange_of(*iother) ) icurr->add_inrange_arrow(*iother);
            for(auto iother=icurr+1; iother!=all_the_arrows.end(); ++iother)
                if( icurr->is_inrange_of(*iother) ) icurr->add_inrange_arrow(*iother);
           }
    };
    

    Then I'd consider if this precalculation is really necessary, and to remove m_inrange from Arrow at all putting it locally just where this information is needed.