0

So I have a list class AnimalCollection which inherits from list<Animal>.

Now I am implementing a method called removeById(int id) which removes certain item in the collection that matches the specified id from the parameter.

My current implementation is below:

void AnimalCollection::removeById(int id)
{
    for (auto it = this->begin(); it != this->end(); it++)
    {
        if (it->id == id)
            this->remove((Animal)it);
    }
}

Now, on line 6 of my code, it's obvious that it's unwise to convert an iterator to the type that is currently being iterated.

The point is, I want to get the current item being iterated by the iterator it.

Anyone who has idea on how to do it?

Blacktempel
  • 3,935
  • 3
  • 29
  • 53
Allen Linatoc
  • 624
  • 7
  • 17
  • 2
    can't you dereference `it`? like `this->remove(*it);` – Sam Aug 03 '15 at 09:16
  • 2
    Are you looking for `*it`? – nwp Aug 03 '15 at 09:16
  • OMG, so that's it, hahah. My ignorance. Thanks for the **dereferencing** part. Saved my day. – Allen Linatoc Aug 03 '15 at 09:18
  • 3
    Word of advice: don't post code with line numbers. If someone copies your code, first the numbers have to be removed before it can be compiled and executed. I'll edit your question accordingly. – rbaleksandar Aug 03 '15 at 09:20
  • 1
    Yes, I'm actually hoping for line numbers to be available in code snipping here so it will be easy to specify a part / some parts in your code instead of copying and pasting again. But it will be frustrating for someone to compile that since I didn't give the source for ``AnimalCollection`` and ``Animal`` objects :D Anyway, thanks a lot mate. – Allen Linatoc Aug 03 '15 at 09:23
  • Side note : inheriting from standard containers [has some pitfalls](http://stackoverflow.com/questions/6806173/subclass-inherit-standard-containers), make sure you know them and avoid them. – Quentin Aug 03 '15 at 09:41

2 Answers2

4

The iterator returned by std::list#begin() is a bidirectional iterator. As such, it's also an input iterator and support the dereference operator (*it) to access the underlying object being iterated :

this->remove(*it);

But note that the call to remove will iterate once again over the list to find and remove all elements that equals to *it.

Alternatively, you could use std::list#erase() to directly remove the iterator (and only the element referenced by it) from the list and avoid a possible useless iteration :

for (auto it = this->begin(); it != this->end(); ) {
    if (it->id == id) {
        it = this->erase(it);
    } else {
        ++it;
    }
}

Note that if you call erase(it), your iterator it will be invalidated, thus in order to continue to iterate the list, we use the iterator returned by erase wich point to the element following the last removed element, possibly end()

zakinster
  • 10,508
  • 1
  • 41
  • 52
  • Glady appreciated for pointing 2 concepts as solutions for my question. Thanks mate (bow) – Allen Linatoc Aug 03 '15 at 09:20
  • 1
    Caution : you should not increment `it` at the end of an iteration if you used `it = erase(it);`. Otherwise, erasing the last element will increment `it` past `end()`. – Quentin Aug 03 '15 at 09:31
  • Caution returns : your fix will shove `it` before `begin()` if you `erase()` the first element ;) – Quentin Aug 03 '15 at 09:42
  • I chose this as **The Best Answer** since he elaborated perfectly and completely, not only the solutions, but also possible countermeasure for another incoming problem. Everyone else, thanks for your effort and time. – Allen Linatoc Aug 03 '15 at 09:43
  • Caution is satisfied and has left an upvote for your time. Enjoy your coffee :) – Quentin Aug 03 '15 at 09:45
2

If all you need is remove the element referred by it then erase(it) is what you need.

Quentin
  • 62,093
  • 7
  • 131
  • 191
marom
  • 5,064
  • 10
  • 14