c++ Iterating over a list of objects and deleting a object

Question

I have a list of objects and I want to delete a certain object once it hits the if condition. But I am having issues with it working. Mostly because the if condition is throwing the error.

Also I don't know if I need to create a temp folder value and let that be my if condition? I'm honestly kind of confused on iterators and any extra information might be helpful.

void removeFolder(string fName)
{
    list<Folder> folders = this->getFolders();
    for (list<Folder> itr = folders.begin(); itr != folders.end();)
    {
        if (fName == *itr)
            itr = folders.erase(itr);
        else
            ++itr;
    }
}

Is a `Folder` the same as a `folder`? And have you written an operator that will allow you to test the equality of `fName` (which is a `string`) and `*itr` (which is a `folder`)? — Beta, Sep 06 '18 at 22:24
You are comparing a `string` with a `Folder`. If you have a function that returns the name of a `Folder`, you can use that. E.g. `if ( fName == (*iter).getName() )`. — R Sahu, Sep 06 '18 at 22:28

score 2 · Accepted Answer · answered Sep 06 '18 at 22:48

2

I think you have correct idea, but you are doing wrong thing. folders is:

list<Folder> folders

Than you are iterating different elements "folder" not "Folder":

for (list<folder> itr = folders.begin(); itr != folders.end();)

I think this should be rather:

for (list<Folder>::iterator itr = folders.begin(); itr != folders.end();)

Now once you are iterating correct objects make comparisons that make sense, not string to object:

if (fName == *itr)

but rather compare string to string, I assume your Folder class has some method to get folder name i.e:

if (fName == (*itr).getFolderName())

answered Sep 06 '18 at 22:48

Mateusz Wojtczak

1,621
1
12
28

This would modify a local variable `folders` which will be destroyed after the function exits. That's not the intent, I am guessing. – PlinyTheElder Sep 06 '18 at 22:51
An alternative is `if (*itr == fName)` and have an `operator==` that compares `Folder` with `string`. [Suggestions on how to write the overload.](https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading) – user4581301 Sep 06 '18 at 23:04

score 0 · Answer 2 · edited Sep 06 '18 at 22:42

0

I'm sure there is this question answered somewhere already, but the code is simple:

You only need to use std::list<Folder>::remove_if().

void removeFolder(const std::string& fName)
{
    std::list<Folder>& folders = getFolders();
    folders.remove_if([&fName](Folder& item){return item.name == fName;});
}

As others have noted, there are other problems with your code that I've tried to fix. In particular, you are probably comparing some sort of name member with the fName variable. Also there is no need to pass the fName string by value into the removeFolder function, and presumably you need to modify not a local copy of folders, but some list existing outside of the function.

edited Sep 06 '18 at 22:42

aschepler

70,891
9
107
161

answered Sep 06 '18 at 22:33

PlinyTheElder

1,454
1
10
15

2

The member function `list::remove_if` is a different thing from the non-member function `std::remove_if`. The member does not return an iterator, and includes the erase operations so does not need to be followed with any `erase()` call. – aschepler Sep 06 '18 at 22:37
Thank you! Good to know. Fixed. – PlinyTheElder Sep 06 '18 at 22:39
1

Since C++17 introduced `std::string_view`, one should prefer passing a `std::string_view` over a `std::string const&` for flexibility and efficiency, unless the 0-terminator is actually needed. Unfortunately, there's no variant with guaranteed 0-terminator. Also, consider wider application of `auto`. – Deduplicator Sep 06 '18 at 22:46

c++ Iterating over a list of objects and deleting a object

2 Answers2