Как правильно передать итератор по ссылке? - PullRequest
Новая
       22

Как правильно передать итератор по ссылке?

0 голосов
/ 04 декабря 2018

У меня есть игра, в которой я проверяю столкновение между пулями и врагами, и храню их как 2 векторных контейнера.Люди говорят, что если вы хотите стереть элемент в цикле for, вам лучше использовать итераторы, и я так и сделал.Но теперь у меня проблема с передачей итератора в функцию.Дело в том, что мне не обязательно стирать элемент, поэтому он должен быть немного сложнее.

Это способ проверки столкновения."CircularCollision" работает нормально, ошибок нет. 

void ResolveColision(Weapon &weap, Map &map)
{
    std::vector<Bullet> bullets = weap.GetBullets();

    if (!bullets.empty())
    {
        for (std::vector<Bullet>::iterator i = bullets.begin(); i != bullets.end(); ++i)
        {
            std::vector<Enemy> enemies = map.GetEnemies();

            if (!enemies.empty())
            {
                for (std::vector<Enemy>::iterator j = enemies.begin(); j != enemies.end(); ++j)
                {
                    if (CircularCollision((*i), (*j)))
                    {
                        weap.DeleteByIndex(i);
                        map.TakeDamageByIndex(j, weap.GetDamage());
                        std::cout << "HIT!\n";
                    }
                }
            }
        }
    }
}

Вот метод, который должен уменьшить здоровье врага: 

void Map::TakeDamageByIndex(std::vector<Enemy>::iterator &itr, int damage)
{
   (*itr).SetHealth((*itr).GetHealth() - damage);
}

Вот метод, который удаляет пулю:

void Weapon::DeleteByIndex(std::vector<Bullet>::iterator &itr)
{
    destroySprite((*itr).GetSprite());
    bullets.erase(itr);
}

Я уверен, что это выглядит ужасно и не должно работать, но я не знаю, как это сделать правильно.Пожалуйста помоги!Кроме того, оба метода работают правильно, когда циклы for работают с индексами (например, маркерами [i]), в этом случае проблема связана с ошибкой «Векторный индекс вне диапазона».

Ответы [ 3 ]

0 голосов
/ 04 декабря 2018

Несколько других комментариев в дополнение к ответу Реми Лебо.

Передача итератора STL по значению так же эффективна, как и по ссылке, поэтому единственная причина, по которой вам нужно будет передать один итератор по ссылке, - это когда вынамереваетесь изменить индекс, и вы хотите, чтобы это изменение было видно в области действия вызывающего.(Например, анализатор UTF-8 должен потреблять от одного до четырех байтов.) Поскольку этот код не должен этого делать, лучше просто передать итератор по значению.

ВВ общем, если вы не изменяете переменную, которую вы передаете по ссылке, вы должны вместо этого передать const ссылку.В случае Enemy::TakeDamage() единственное, что вы делаете с итератором, это разыменовываете его, так что вы можете просто передать Enemy& и вызвать его с *i в качестве параметра.

Алгоритм не очень эффективен: если вы удаляете много элементов в начале списка, вам нужно будет переместить все оставшиеся элементы массива несколько раз.Это работает в O(N²) время.A std::list, хотя и имеет большие накладные расходы по сравнению с std::vector, может удалять элементы за постоянное время и может быть более эффективным, если у вас много вставок и удалений, которые не находятся в конце.Вы также можете рассмотреть возможность перемещения только тех объектов, которые сохранились, в новый список, а затем уничтожить старый.По крайней мере, таким образом, вам нужно копировать только один раз, и ваш проход выполняется за O(N) раз.

Если ваши контейнеры хранят умные указатели на объекты, вам нужно только переместить указатели на новое место,не весь объект.Это не компенсирует накладные расходы большого количества выделений кучи, если ваши объекты маленькие, но может сэкономить вам большую пропускную способность, если они большие.Объекты по-прежнему будут автоматически удаляться при удалении последней ссылки на них.

0 голосов
/ 04 декабря 2018

Вы можете сделать что-то вроде этого: 

void delByIndex(vector<int>::iterator &i, vector<int>& a)
{
    a.erase(i);
}   

int main()
{

    vector<int> a {1,5,6,2,7,8,3};
    vector<int> b {1,2,3,1};

    for(auto i=a.begin();i!=a.end();)
    {
        bool flag = false;
        for(auto j=b.begin();j!=b.end();j++)
            {
                if(*i==*j)
                {                    
                    flag = true;
                }
            }
            if(flag)
            {
                delByIndex(i, a);
            }
            else
                i++;

    }

    for(auto i:a)    
        cout << i << " ";


    return 0;
}

Будьте осторожны при использовании стирания, так как это изменит размер вектора, а также сделает недействительным векторный итератор.

0 голосов
/ 04 декабря 2018

В DeleteByIndex() измените это: 

bullets.erase(itr);

На это: 

itr = bullets.erase(itr);

std::vector::erase() возвращает итератор для следующего оставшегося элемента послеэлемент, который был стерт.Этот следующий элемент - это то место, с которого ваш внешний цикл должен продолжить свою следующую итерацию.

Таким образом, вам нужно вместо этого изменить внешний цикл с for на while, иначе вы пропуститеelements (фактически ваш исходный код страдает от этой проблемы, когда вы все еще используете индексы): 

void ResolveColision(Weapon &weap, Map &map)
{
    std::vector<Bullet> bullets = weap.GetBullets();

    std::vector<Bullet>::iterator bullerItr = bullets.begin();
    while (bullerItr != bullets.end())
    {
        std::vector<Enemy> enemies = map.GetEnemies();
        bool wasAnyHit = false;

        for (std::vector<Enemy>::iterator enemyItr = enemies.begin(); enemyItr != enemies.end(); ++enemyItr)
        {
            if (CircularCollision(*bulletItr, *enemyItr))
            {
                wasAnyHit = true;
                weap.DeleteByIndex(bulletItr);
                map.TakeDamageByIndex(enemyItr, weap.GetDamage());
                std::cout << "HIT!\n";
                break;
            }
        }

        if (!wasAnyHit)
            ++bulletItr;
    }
}

При этом я бы предложил вместо этого заменить внутренний цикл на std::find_if().И переименование DeleteByIndex() и TakeDamageByIndex(), так как они больше не берут индекс.На самом деле, я бы вообще не передавал итератор в TakeDamage...(), вместо этого передавал бы фактический Enemy объект.Или лучше, переместите TakeDamage() в Enemy сам.

Попробуйте что-то вроде этого: 

void ResolveColision(Weapon &weap, Map &map)
{
    auto bullets = weap.GetBullets();

    auto bulletItr = bullets.begin();
    while (bulletItr != bullets.end())
    {
        auto enemies = map.GetEnemies();
        auto &bullet = *bulletItr;

        auto enemyHit = std::find_if(enemies.begin(), enemies.end(),
          [&](Enemy &enemy){ return CircularCollision(bullet, enemy); }
        );

        if (enemyHit != enemies.end())
        {
            weap.DeleteBulletByIterator(bulletItr);
            enemyHit->TakeDamage(weap.GetDamage());
            std::cout << "HIT!\n";
        }
        else
            ++bulletItr;
    }
}

void Enemy::TakeDamage(int damage)
{
   SetHealth(GetHealth() - damage);
}

void Weapon::DeleteBulletByIterator(std::vector<Bullet>::iterator &itr)
{
    destroySprite(itr->GetSprite());
    itr = bullets.erase(itr);
}
...