CODEWITHSUNDEEP
c++vectorcompiler-errors
Dj Agor
Dj Agor

Reputation: 13

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

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.

Upvotes: 0

Views: 3991

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 595295

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;
}

Upvotes: 2

Related Questions