No matching member function for call to 'erase' while erasing a value from vector

Question

I am trying to write this code but this is giving the error

No matching member function for call to 'erase'clang(ovl_no_viable_member_function_in_call)
stl_vector.h(1317, 7): Candidate function not viable: no known conversion from 'int' to 'std::vector<int, std::allocator<int> >::const_iterator' (aka '__normal_iterator<const int *, std::vector<int, std::allocator<int> > >') for 1st argument
stl_vector.h(1344, 7): Candidate function not viable: requires 2 arguments, but 1 was provided

Here is my Code

#include <iostream>
#include <vector>

using namespace std;


int sockMerchant(int n, vector<int> ar) {
    int pairCount=0;
    for(int i=0;i<ar.size();i++)
    {
        for(int j=i+1;j<ar.size();j++)
        {
            if(ar[i]==ar[j])
            pairCount++;
            ar.erase(j);
        }
    }
    return pairCount;
}

int main()
{
    int n;
    cin>>n;
    vector <int> arr;
    for(int i=0;i<n;i++)
    {
        int e=0;
        cin>>e;
        arr.push_back(e);
    }
    sockMerchant(n, arr);
}

Also if you can, please tell me why am I getting this error as I followed the same approach of erase function as it was done in gfg.

Errors like that when using standard functions are an indication that you should consult your favorite documentation about the function you're trying to call ([`erase`](https://en.cppreference.com/w/cpp/container/vector/erase), in this case). — 1201ProgramAlarm, Nov 18 '20 at 22:27
So there is no way or workaround to erase the element as I did? — Dj Agor, Nov 18 '20 at 22:34
Vector does not allow to erase elements by index. For that, need to use iterators - but be careful since iterators become invalid after erase() call. Depending on what exactly you are trying to achieve, there may already be STL algorithms to do it. If you describe what it is, somebody may be able to help. — Eugene, Nov 19 '20 at 00:04
You can't erase elements by index, only by iterator. Be careful erasing elements in a loop, though. `erase()` invalidates iterators, and decrements the `size()`. Plenty of questions on StackOverflow related to this topic, such as [C++ nested for loop with erasing elements](https://stackoverflow.com/questions/54038997/), [Remove elements of a vector inside the loop](https://stackoverflow.com/questions/8628951/), [Erase in a loop with a condition in C++](https://stackoverflow.com/questions/38356875/), etc — Remy Lebeau, Nov 19 '20 at 00:59
I'm having a hard time trying to figure out what the intent of `sockMerchant()` really is. It is full of logic issues that make it difficult to correct without knowing the actual goal. — Remy Lebeau, Nov 19 '20 at 01:19
Given an array of integers representing the color of each sock, determine how many pairs of socks with matching colors there are. This is the question so to avoid extra pairs, I wanted to remove the element which have been paired (cuz, in the end, I just need pairCount). — Dj Agor, Nov 19 '20 at 05:07

Remy Lebeau · Accepted Answer · 2020-11-19T05:26:56.023

std::vector::erase() does not take an index as input. That is what the compiler is complaining about. erase() takes an iterator instead, eg:

ar.erase(ar.begin()+j);

Note that you have to take care, because erase() will invalidate iterators and references to elements at/after the element being erased. But more importantly, it also decrements the vector's size(), which you are not accounting for correctly in your inner loop, so it will skip elements.

For that matter, your outer loop is pretty useless. It will iterate only 1 time at most, because the inner loop erases all elements after the 1st element. Is that what you really want? If your intention is just to count elements, there is no need to erase them at all.

Also, you are passing the vector by value, which will make a copy of the vector. Any modifications you make to the copy will not be reflected back to the caller. If you want to modify the caller's original vector, you need to pass it by non-const reference instead.

UPDATE: given what you have said about your intended goal, try something more like this instead:

int sockMerchant(vector<int> ar) {
    int pairCount = 0;
    for(size_t i = 0; i < ar.size(); ++i) {
        for(size_t j = i + 1; j < ar.size(); ++j) {
            if (ar[i] == ar[j]) {
                ++pairCount;
                ar.erase(ar.begin()+j);
                break;
            }
        }
    }
    return pairCount;
} 
...
sockMerchant(arr);

Alternatively, another option is to sort the array first, then use 1 loop to count the now-adjacent pairs, without erasing any elements, eg:

#include <algorithm>

int sockMerchant(vector<int> ar) {
    sort(ar.begin(), ar.end());
    int pairCount = 0;
    for(size_t i = 1; i < ar.size(); ++i) {
        if (ar[i-1] == ar[i]) {
            ++pairCount;
            ++i;
        }
    }
    return pairCount;
}

I want to count the number of pairs in an array. So, to avoid three elements being counted as 2 pairs, I wanted to erase the element from the array so that it can not be counted again. — Dj Agor, Nov 19 '20 at 05:04
@DjAgor "*I want to count the number of pairs in an array*" - but that is not what your code is actually doing, though. It is taking the 1st element, counting how many times that element is duplicated through the entire array, and erasing ALL elements after the 1st element. At the very least, your code is missing `{}` brackets and a `break` in the `if(ar[i]==ar[j])` block. I would suggest instead `std::sort()`'ing the array and then counting the adjacent pairs with 1 loop without erasing any elements at all. — Remy Lebeau, Nov 19 '20 at 05:09
Yes, I was about to include the edit. And your approach is sounding better to me. It just clicked me using erase so I used that. Thank you — Dj Agor, Nov 19 '20 at 05:16

No matching member function for call to 'erase' while erasing a value from vector

1 Answers1