Removing Values From a Vector

I am currently writing some generic functions to make working with vectors a little "cleaner". Working with the "erase-remove idiom" can be a little confusing sometimes, and make the code look a little dirty, so why not throw it into a function? I have four individual function I wish to have reviewed and made better. I know that there are tons of examples online (trust me, I've looked at most of them).

1. Simply Remove All Instances of a Value From a Vector

template
void Vec_RemoveAll(vector& vec, T val)
{
    vec.erase(remove(vec.begin(), vec.end(), val), vec.end());
}

This function is the simplest of all four.

1. Remove All Instances of a Container of Values From a Vector

template
void Vec_RemoveAll(vector& vec, const Container& vals)
{
    for (const T& val : vals) vec.erase(remove(vec.begin(), vec.end(), val), vec.end());
}

Simply loops through the container checking the vector for occurrences of each value.

1. Remove All Elements That Meet Unary Predicate's Condition

template
void Vec_RemoveAll_If(vector& vec, UnaryPredicate* predicate)
{
    vec.erase(remove_if(vec.begin(), vec.end(), predicate), vec.end());
}

I'm not familiar with remove_if, I'm worried I may have not implemented this correctly.

1. Remove Elements That Meet Any of the Predicates' Conditions

template
void Vec_RemoveAll_If(vector& vec, const vector predicates)
{
    for (const UnaryPredicate& predicate : predicates)
        vec.erase(remove_if(vec.begin(), vec.end(), predicate), vec.end());
}

How would one generalize the type of container that the unary predicates are stored in?

Before getting into each function, your last question is important -- how do we generalize the type of container? By passing iterators instead of containers, of course! When we first encounter the C++ Standard Library, the ubiquity of iterators can seem confusing -- why not pass containers around instead? The reason is that different programs have many different needs, and passing a particular kind of container can be very restricting. For example, if my function takes a parameter of type const vector&, I cannot pass a const vector& or a const vector&. If instead I take a pair of iterators with the requirement that *it is of type const Foo&, I can use a wider range of container types; vector's and vector's iterators work directly, and vector can be made to work with iterator adaptors.

OK, function by function.

1. Simply Remove All Instances of a Value From a Vector

template
void Vec_RemoveAll(vector& vec, T val)
{
    vec.erase(remove(vec.begin(), vec.end(), val), vec.end());
}

First I'll include my standard disclaimer about using namespace std;: Don't do it, it can lead to subtle bugs, and you'll get used to seeing std:: everywhere.

Second, pass const T&, not T. What if T is a large type? std::remove takes const T&.

Third, I don't think this is a particularly wise function to write. It's a common C++ idiom; once you learn it it should become automatic.

1. Remove All Instances of a Container of Values From a Vector

template
void Vec_RemoveAll(vector& vec, const Container& vals)
{
    for (const T& val : vals) vec.erase(remove(vec.begin(), vec.end(), val), vec.end());
}

First point is a question: Is this the performance characteristic you want? Intuitively, I would think that vals would tend to be smaller than vec, so you probably want to iterate through vals in the inner loop, not the outer loop. You could achieve this by using std::remove_if instead and writing a function that determines whether an element is in vals. You could get even better performance if you required vals to be sorted.

Second: for (const T& val : vals) is dangerous. Container isn't required to contains Ts, just things that are comparable to T. You would want to write this as for (const auto& val : vals). For example, consider that vec is a vector and vals is a vector. Each of the strings in vals would be copied into a string while iterating using your version.

1. Remove All Elements That Meet Unary Predicate's Condition

template
void Vec_RemoveAll_If(vector& vec, UnaryPredicate* predicate)
{
    vec.erase(remove_if(vec.begin(), vec.end(), predicate), vec.end());
}

Almost right, except that you should pass UnaryPredicate, not UnaryPredicate*. Your version works fine for function pointers but not function objects. But I raise the same objection as to function 1 -- this is a standard idiom, and hiding it behind a function call is more likely to confuse than enlighten experience C++ programmers who read your code.

1. Remove Elements That Meet Any of the Predicates' Conditions

template
void Vec_RemoveAll_If(vector& vec, const vector predicates)
{
    for (const UnaryPredicate& predicate : predicates)
        vec.erase(remove_if(vec.begin(), vec.end(), predicate), vec.end());
}

I recommend against writing this function in the first place; it's probably best for any potential user of this function to compose their predicates manually into a single predicate, or in the alternative to write a library to help them do so. That is more generally applicable and removes the requirements that users of your library learn this idiosyncratic API. In the alternative, you can do as you did in function 2, and pass const Container& instead of trying to specify a specific container.

StackExchange Code Review Q#157109, answer score: 5