Search code examples
c++singletonsdlstate-machine

How to delete an element of a vector through a getter function?


I am new to C++ and I am working on a game that is using a Singleton design pattern and various state machines.Currently I have most of my games update information within my Engine class update function and I need to move the majority of code over into the Update function in my gamestate class.

Some of the code I need to move manages and deletes enemies in my enemy vector as seen below. Since I am accessing some of these vectors outside of the class I am using the getter function below. I am trying to delete my enemies if they travel off the screen.This compiles however when an enemy travels off the screen the following Unhandled exception is thrown: _Mylast was 0xDDDDDDE5. Any help that can be provided is really appreciated.

for (int i = 0; i < (int)m_vEnemies.size(); i++)
    {
        m_vEnemies[i]->Update(); 
        if (m_vEnemies[i]->GetDstP()->x < -56)
        {
            delete m_vEnemies[i];
            m_vEnemies[i] = nullptr;

        }
    }
vector<Enemy*> Engine::getEnemies()
{
    return m_vEnemies;
}
for (int i = 0; i < (int)Engine::Instance().getEnemies().size(); i++)
    {
        Engine::Instance().getEnemies()[i]->Update(); 
        if (Engine::Instance().getEnemies()[i]->GetDstP()->x < -56)
        {
            delete Engine::Instance().getEnemies()[i];
            Engine::Instance().getEnemies()[i] = nullptr;

        }
    }

Solution

  • You're deleting the object pointed to by m_vEnemies[i] and setting m_vEnemies[i] to nullptr, but the next time you iterate through the array, that nullptr is still there and you'll try to dereference it. You also need to erase the item from the vector. Items are erased from vectors by using iterators.

    However, you can't just iterate through a vector using standard for (auto it = v.begin(); it != v.end(); it++) semantics and erase the item in the middle of the loop, because erasing an item from a vector invalidates its iterators.

    The trick is to get in iterator to the item you're removing using

    std::vector<T *>::iterator erase_iterator = v.begin() + i;
    

    which you can then use to remove the item from the vector after appropriately deleting it.

    When you find an item to delete you:

    1. delete the allocated memory
    2. Decrement your loop variable. On the next loop iteration, you want to check the new item that occupies this same memory address
    3. Acquire an iterator of the appropriate type that identifies the current item
    4. Call std::vector::erase(iterator) to erase the item from the vector.

    Not that this causes a reshifting of all pointers in the vector, since items are stored in an array contiguously.

    #include <iostream>
    #include <vector>
    
    
    void print_container_info(std::vector<int *>& container)
    {
        std::cout << "Container size: " << container.size() << "\n";
        std::cout << "Container contents: ";
        for (auto& item : container)
        {
            std::cout << *item << " ";
        }
    }
    
    int main(int argc, char** argv)
    {
        std::vector<int*> p_int_vec = { new int(3), new int(1), new int(2), new int(2), new int(3), new int(3), new int(2) };
        print_container_info(p_int_vec);
        std::cout << "\n\n";
    
        std::cout << "Removing all elements equal to 2 using for loop\n";
        for (std::size_t i = 0; i < p_int_vec.size(); i++) 
        {
            if (*p_int_vec[i] == 2)
            {
                auto erase_iterator = p_int_vec.begin() + i;
                delete p_int_vec[i];
                p_int_vec.erase(erase_iterator); //no need to set to nullptr, we're removing it from the container
                --i; // must decrement
            }
        }
        print_container_info(p_int_vec);
        std::cout << "\n\n";
    }
    

    You could also include the algorithm library, and use the std::remove_if function. remove_if applies a predicate on each item of the container, and moves the item to the end of the container (potentially invalidating its data) if a criteria is met. Since the pointer may be invalidated after moving it to the end of the container, we need to delete it before the move (i.e., before we return true from the functor).

    Then, the container contains all valid elements at the start of the vector, with order preserved, and all invalidated elements at the end. remove_if returns an iterator to the start of the range where the invalidated elements start. After calling remove_if, you should call std::erase on the range between this dummy-start iterator and the end of the container.

    #include <iostream>
    #include <vector>
    #include <algorithm>
    
    void print_container_info(std::vector<int *>& container)
    {
        std::cout << "Container size: " << container.size() << "\n";
        std::cout << "Container contents: ";
        for (auto& item : container)
        {
            std::cout << *item << " ";
        }
    }
    
    int main(int argc, char** argv)
    {
        std::vector<int*> p_int_vec = { new int(3), new int(1), new int(2), new int(2), new int(3), new int(3), new int(2) };
        print_container_info(p_int_vec);
        std::cout << "\n\n";
    
        std::cout << "Removing all elements equal to 2 using for loop\n";
        auto dummy_begin = std::remove_if(p_int_vec.begin(), p_int_vec.end(),
            [](int* p_int) {
                if (*p_int == 2)
                {
                    delete p_int;
                    return true;
                }
                return false;
            });
        p_int_vec.erase(dummy_begin, p_int_vec.end());
        print_container_info(p_int_vec);
        std::cout << "\n\n";
    }