Search code examples
c++remove-iferase-remove-idiom

remove_if erases everything from vector


I'm writing a program for an assignment - it's supposed to be a database for information about employees in a company. Basically, a vector containing structures (individual employees).

The trouble I'm having is that remove_if erases everything from the vector - instead of an individual employee.

If I understood documentation/other topics correctly, that function should do two things - rearrange elements of the vector, and return an iterator to the first element outside the new range - but it doesn't do it, it returns an iterator to the first element - and so when the erase() function is called, all elements are deleted. At least that's what I found when debugging it.

Here's a mcve of my code:

#include <iostream>
#include <vector>
#include <algorithm>
struct employee {
    int number;
};
int main()
{
    //creating the vector and adding some values to it
    employee one{ 1 };
    employee two{ 2 };
    employee three{ 3 };
    std::vector <employee> staff{ one, two, three };


    int m = 2; //some parameter I want to pass to lambda function
    auto it = std::remove_if(staff.begin(), staff.end(),
        [m](employee a) {
        if (a.number == 2)
            return true; }
    );
    staff.erase(it, staff.end());

    for (auto it = staff.begin(); it != staff.end(); it++)
        std::cout << it->number << std::endl;
    system("pause");
    return 0;
}

I realise that I could've done the same thing in a loop - in fact, I did, but I just can't wrap my head around why doesn't this approach work. Also, a list would've probably been a better choice for this program (with it, the for loop would have taken fewer instructions to compute), but I've already finished the program, and right now I just really want to know why didn't remove_if work.

Thanks!

EDIT: As @drescherjm pointed out, that was due to the fact that the lambda function didn't return false when the when the if statement wasn't met.

So the question is answered.


Solution

  • The main problem is you are not returning a value when your condition in your lambda is not met. This is undefined behavior not to return a value.

    auto it = std::remove_if(staff.begin(), staff.end(),
            [m](employee a) {
            if (a.number == 2)
                return true; }
    );
    

    A simple solution is to remove the if and just return the conditional.

    auto it = std::remove_if(staff.begin(), staff.end(),
            [m](employee a) {
            return (a.number == 2);
            }
    );
    

    However as @killzonekid mentioned this is not correct because you are still not using the parameter.

    auto it = std::remove_if(staff.begin(), staff.end(),
            [m](employee a) {
            return (a.number == m);
            }
    );
    

    Replacing the fixed 2 with m should take care of that.