Странное поведение с std::vector в С++
Почему это не работает?
Это базовая система инвентаризации. inventory — это std::vector<Item>, а Item — это структура с int quantity и 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);
}
}
}
Я ожидал, что это сработает. Когда он пуст, добавьте один к inventory из name, а когда он не пуст, просто добавьте еще один. Если он существует, просто добавьте 1 к quantity.
Вы вставляете новый элемент на каждой итерации цикла.
Ответы 2
Вам не нужна первая проверка empty(), цикл прекрасно обработает пустую vector.
Ваш цикл неправильный, потому что он помещает новый Item в vector на каждой итерации (что является неопределенным поведением для цикла range-for).
Вообще не вызывайте push_back() внутри цикла. Сначала пройдитесь по vector в поисках нужного Item. Если он найден, вы можете обновить его и прекратить цикл. Если он не найден, добавьте его после завершения цикла.
Попробуй это:
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";
}
ОБНОВЛЕНИЕ: При этом рассмотрите возможность полного удаления ручного цикла и вместо этого используйте std::find_if (), например:
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";
}
}
Возможно, стоит процитировать документацию по push_back. Например, на странице cppreference.com в push_back говорится: Если после операции новый размер() превышает старый емкость(), происходит перераспределение, и в этом случае все итераторы (включая итератор end()) и все ссылки на элементы становятся недействительными. В противном случае только итератор end() будет признан недействительным.
@wohlstad Диапазон, основанный на цикле for, — это не то же самое, что необработанный цикл for, ИМХО. Для таких простых вещей я предпочитаю использовать их, поскольку их легче читать.
Проблема в том, что вы не можете добавить элемент в контейнер (как вы это делаете с push_back) во время итерации по нему на основе диапазона. На самом деле вам следует добавить его после цикла (и только если элемент не найден).
Однако в этом случае вы можете вообще избежать цикла и использовать std::find_if из заголовка <algorithm>.
Он выполнит поиск в контейнере и вернет итератор элементу (если он найден) или итератор end (если нет).
Лямбда используется для проверки того, соответствует ли имя элемента имени элемента в контейнере.
Это показано ниже:
#include <algorithm> // required for std::find_if
void AddItemToInventory(std::string const & itemName)
{
auto it = std::find_if (inventory.begin(),
inventory.end(),
[&itemName](Item const& item) { return item.name == itemName; });
if (it == inventory.end())
{
Item i;
i.name = itemName;
i.quantity = 1;
inventory.push_back(i);
std::cout << "Added\n";
}
else
{
it->quantity += 1;
std::cout << "Updated\n";
}
}
Обратите внимание, что я изменил itemName на const&, чтобы избежать ненужной копии при передаче AddItemToInventory.
Демо - Godbolt .
Изменение контейнера во время итерации по нему с помощью цикла с диапазоном for приводит к UB. В любом случае, похоже, вам нужен
std::map, а неstd::vector(но отладка проблемы с помощьюvector, вероятно, имеет обучающую ценность).