remove_if erases everything from vector

Question

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.

Your lambda does not return false. You don't have a return value if your condition is not met. This is Undefined Behavior. — drescherjm, Dec 26 '17 at 23:07
Good point. The original code from the OP made no use of the value of m. — drescherjm, Dec 26 '17 at 23:34

score 2 · Accepted Answer · answered Dec 26 '17 at 23:40

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.

remove_if erases everything from vector

1 Answers1