5

I have the following code in C++:

#include <iostream>
#include <cstdlib>
#include <ctime>
#include <vector>
int main ()
{
    srand(time(0));
    int noOfElements = 9;
    for (int a = 0; a < 9; a++)
    {
        std::vector<int> poss;
        for (int a = 1; a <= 9; a++)
            poss.push_back(a);
        for (int b = 0; b < 9; b++)
        {
            int random = rand() % 9;
            std::cout << poss[random];
            poss.erase(random);
            noOfElements--;
        }
        std::cout << "\n";
    }
}

Yet when I run it, it returns this:

error: no matching function for call to 'std::vector<int>::erase(int&)'

for line 13.

Why is this and how can I correct it?

LazySloth13
  • 2,369
  • 8
  • 28
  • 37
  • 1
    http://en.cppreference.com/w/cpp/container/vector/erase – hmjd Apr 25 '13 at 15:39
  • Are you trying to erase a random position, or a random value? It makes quite a big difference. – john Apr 25 '13 at 15:40
  • This would be a lot shorter and more robust if you used standard algorithms. `std::iota` replaces your first inner loop, and `std::random_shuffle` shuffles them. `std::copy` can output them, and `std::vector::clear` can reset the vector. – chris Apr 25 '13 at 15:41
  • 1
    Once `erase` starts working, you'll need to change `rand() % 9` to `rand() % noOfElements`. – Pete Becker Apr 25 '13 at 15:42
  • I hadn't come across `std::iota`. Neat. I've yet to find a full, complete and easy-to-read list of everything new in C++11... – Chowlett Apr 25 '13 at 15:54

3 Answers3

13

You cannot erase values from a vector directly (vectors are sequence containers, not associative containers): you need to provide an iterator to the element that you want to be erased.

In order to get an iterator, you may either:

  • find the element based on its value (e.g. by using std::find()) and then provide the returned iterator in input to the erase() member function, or
  • get it by applying an offset to the iterator which points to the beginning of your vector (i.e. the object returned by the begin() member function).

In the first case:

#include <vector>
#include <algorithm>

int main()
{
    std::vector<int> v { 1, 2, 3};
    auto i = std::find(begin(v), end(v), 2);
    v.erase(i);
}

The above code uses some C++11 features. In C++03, it would look as follows:

#include <vector>
#include <algorithm>

int main()
{
    std::vector<int> v;

    v.push_back(1);
    v.push_back(2);
    v.push_back(3);

    std::vector<int>::iterator i = std::find(v.begin(), v.end(), 2);
    v.erase(i);
}

In the second case, if you know the index of your element inside the vector (say, pos), then you can easily get an iterator this way:

v.begin() + pos

Alternatively (C++11 only) you could do:

next(begin(v), pos);
Andy Prowl
  • 124,023
  • 23
  • 387
  • 451
  • In this case, I think he'd most likely want `std::vector::iterator it = poss.begin() + random; std::cout << *it; poss.erase(it);` – Chowlett Apr 25 '13 at 15:42
  • @Chowlett: Re-reading the question, it seems you are right. Initially I thought `random` was the *value* the OP wanted to erase, not its position. But that's probably not the case, so I updated my answer to cover both situations. Thank you – Andy Prowl Apr 25 '13 at 15:50
6

you have to pass an iterator to erase. So try

poss.erase(poss.begin() + random);
TooTone
  • 7,129
  • 5
  • 34
  • 60
  • I think this is a the correct answer, but Andy Prowl could be right instead. – john Apr 25 '13 at 15:41
  • @john yes, I agree, I was going to edit my post to say that I wasn't sure what the OP's intention is. I also have a concern that the OP's loop is going to go past the end of the array -- I'd want to analyse the logic a bit more to convince myself that it's ok (or maybe just add something to check, e.g. on the lines of `assert(random < poss.size())` – TooTone Apr 25 '13 at 15:44
0

Vector erase function takes iterator not value. And also you need to check for boundary condition to see the index you are erasing is not out of bound.

std::vector<int>::iterator itr = poss.begin() + random;
if(itr != poss.end())
{
  poss.erase(itr);
}
shivakumar
  • 3,297
  • 19
  • 28
  • The bounds check you're doing is woefully inadequate. It checks the iterator against one possible invalid value. Of course, it would be undefined behavior to form any iterator farther out of bounds than that. So, if you really want to do a bounds check, you should do it on the integer, before you add it to `poss.begin()`. Of course, if you're in complete control of the generation of the integer, as is the case here, that's where the value of the integer should be bound: `int random = rand() % poss.size();`. Any additional checks are redundant. – Benjamin Lindley Apr 25 '13 at 16:37