Weird behaviour with std::vector in c++

Question

Why does this not work?

It's a basic inventory system. inventory is std::vector<Item> and Item is a struct with int quantity and std::string name.

struct Item
{
    int quantity;
    std::string name;
};

void AddItemToInventory(std::string itemName)
{
    Item i;
    i.name = itemName;
    if(inventory.empty())
    {
        i.quantity = 1;
        inventory.push_back(i);
        std::cout << "EMPTY\n";
    }
    else
    {
        for (auto& item : inventory)
        {
            if(i.name == item.name)
            {
                i.quantity = item.quantity + 1;
            }
            else
            {
                i.quantity = 1;
            }
            inventory.push_back(i);
        }
    }
}

I was expecting it to work. When it's empty, add one to inventory of name, and when it's not empty just add another. If there exists one, just add 1 to quantity.

Answer 1

You don't need the first empty() check, the loop will handle an empty vector just fine.

Your loop is wrong because it is pushing a new Item into the vector on every iteration (which is Undefined Behavior for a range-for loop).

Don't call push_back() inside the loop at all. First loop through the vector looking for the desired Item. If it is found, you can update it and stop looping. If it is not found, add it after the loop has finished.

Try this:

void AddItemToInventory(std::string itemName)
{
    for (auto& item : inventory)
    {
        if (item.name == itemName)
        {
            item.quantity += 1;
            std::cout << "UPDATED\n";
            return;
        }
    }

    Item newItem;
    newItem.name = itemName;
    newItem.quantity = 1;

    inventory.push_back(newItem);

    std::cout << "ADDED\n";
}

UPDATE: That being said, consider removing the manual loop altogether and use std::find_if() instead, eg:

void AddItemToInventory(std::string itemName)
{
    auto found = std::find_if(
        inventory.begin(), inventory.end(), 
        [&](const Item &item){ return item.name == itemName; }
    );

    if (found != inventory.end())
    {
        found->quantity += 1;
        std::cout << "UPDATED\n";
    }
    else 
    {
        Item newItem;
        newItem.name = itemName;
        newItem.quantity = 1;

        inventory.push_back(newItem);

        std::cout << "ADDED\n";
    }
}