C++ Assertion Failed: cannot increment value-initialized map/set iterator

Question

I have been doing some training in C++, and last time I tried to run my code on another computer. There, I build in debug, and the execution stopped due to an assertion failed. I am manipulating containers, and when I use map, it seems removing an element is not possible. The removeItem() methode is triggering the assertion, but I could not get why.

Main:

int main()
{
    // Exercice 4.1   Map and strings
    std::list<std::string> list = {"eggs", "milk", "sugar", "chocolate", "flour"};
    CMapStrings mapStrings;
    mapStrings.print();
    mapStrings.addItem(list);
    mapStrings.print();
    mapStrings.addItem("coffee");
    mapStrings.print();
    mapStrings.replaceItem("sugar", "honey");
    mapStrings.print();
    mapStrings.removeItem("milk");  //buggy
    mapStrings.print();
    std::cout << std::endl; 
}

Hpp:

class CMapStrings
{
public:
    CMapStrings();

    void print();

    void addItem(std::string f_item);

    void addItem(std::list<std::string> f_items);

    void removeItem(std::string f_item);

    void removeLastItem();

    void replaceItem(std::string f_previousItem, std::string f_nextItem);

private:
    std::map<int, std::string> m_shoppingList2;
};

Cpp:

CMapStrings::CMapStrings()
{
}

void CMapStrings::addItem(std::string f_item)
{
    m_shoppingList2.insert(std::pair<int, std::string>(m_shoppingList2.size(), f_item));
}

void CMapStrings::addItem(std::list<std::string> f_items)
{
    for (std::uint32_t i = 0; i < f_items.size(); i++)
    {
        auto l_front = f_items.begin();
        std::advance(l_front, i);
        m_shoppingList2.insert(std::pair<int, std::string>(i, *l_front));
    }
}

void CMapStrings::removeItem(std::string f_item)
{
    for(auto it = m_shoppingList2.begin(); it != m_shoppingList2.end(); it++)
    {
        if(it->second == f_item)
        {
            m_shoppingList2.erase(it->first);
        }
    }
}

void CMapStrings::replaceItem(std::string f_previousItem, std::string f_nextItem)
{
    for(auto it = m_shoppingList2.begin(); it != m_shoppingList2.end(); it++)
    {
        if(it->second == f_previousItem)
        {
            it->second = f_nextItem;
        }
    }
}

void CMapStrings::print()
{
    std::cout << "shopping list size (map): " << m_shoppingList2.size() << std::endl;
    std::cout << m_shoppingList2 << std::endl;
}

Answer 1

Rewrite this for loop

for(auto it = m_shoppingList2.begin(); it != m_shoppingList2.end(); it++)
{
    if(it->second == f_item)
    {
        m_shoppingList2.erase(it->first);
    }
}

the following way

for(auto it = m_shoppingList2.begin(); it != m_shoppingList2.end(); )
{
    if(it->second == f_item)
    {
        it = m_shoppingList2.erase(it);
    }
    else
    {
        ++it;
    }
}

Pay attention to that this statement

std::cout << m_shoppingList2 << std::endl;

does not make a sense if you did not define the operator << for objects of the type std::map.

And this loop

for (std::uint32_t i = 0; i < f_items.size(); i++)
{
    auto l_front = f_items.begin();
    std::advance(l_front, i);
    m_shoppingList2.insert(std::pair<int, std::string>(i, *l_front));
}

is inefficient. You could use the range-based for loop for example like

int i = 0;

for ( const auto &s : f_items )
{
    m_shoppingList2.insert(std::pair<int, std::string>( i++, s ) );
}