-1

I tried to remove duplicate elements from a vector by a function vectorremove, using the function remove from the library of algorithms, but it does not work:

#include <iostream>
#include <vector>
#include <algorithm>
#include <string>
using namespace std;

void vectorremove(vector<string> v)
{
    for (vector<string>::iterator it = v.begin(); it != v.end(); ++it)
    {
        vector<string>::iterator end = remove(it + 1, v.end(), *it);
        v.erase(end, v.end());
    }
}

int main()
{
    vector<string> vect;
    string x;

    while (cin >> x)
    {
        vect.push_back(x);
    }

    vectorremove(vect);

    for (vector<string>::iterator it = vect.begin(); it != vect.end(); ++it)
    {
        cout << *it << endl;
    }
    return 0;
}

I wrote this code to test if the function vectorremove works, unfortunately it seems that vectorremove has no impact on the vector. Have I made any mistake in the use of remove in the definition of vectorremove?

NutCracker
  • 11,485
  • 4
  • 44
  • 68
Richard
  • 131
  • 3
  • 5
    `it` is invalidated by the `erase` operation, instead of doing `++it` you should use the return value of `erase` – M.M Feb 24 '20 at 01:17
  • 6
    Also you pass the vector by value to the vectorremove function, so `vect` in `main` is unchanged – M.M Feb 24 '20 at 01:17
  • @M.M Can you please elaborate how to use the return value of ```erase``` instead of ```++it```? – Richard Feb 24 '20 at 01:23
  • Is there anything that prevents you from opening your C++ book to the chapter that shows you how to use `erase`(), and read the examples that use `erase()`'s return value, by yourself? This is something basic that should be explained in every C++ book, and stackoverflow.com is not a replacement for a C++ book. – Sam Varshavchik Feb 24 '20 at 01:31
  • 1
    Look up "erase-remove idiom". Or read documentation for `vector::erase` – M.M Feb 24 '20 at 01:37
  • Does this answer your question? [Why std::vector iterator is invalidated after the erase() call?](https://stackoverflow.com/questions/40899608/why-stdvector-iterator-is-invalidated-after-the-erase-call) – L. F. Feb 24 '20 at 02:28

3 Answers3

2

The first problem in your code is that you are passing the vector by value and not by reference to vectorremove. You need to change that to

void vectorremove(vector<string>& v);

Then inside your vectorremove function you have another problems. vector::erase can invalidate all iterators, so you should onle remove inside the loop and do the erase after the loop.

void vectorremove(vector<string>& v)
{
    vector<string>::iterator end{ v.end() };
    for (vector<string>::iterator it = v.begin(); it != end; ++it)
    {
        end = remove(it + 1, end, *it);
    }
    v.erase(end, v.end());
}
Werner Henze
  • 16,404
  • 12
  • 44
  • 69
0

First you are passing std::vector by value, not by reference. Therefore, any changes you make in vectorremove function won't be visible in main. Furthermore, std::vector::erase might invalidate iterators so you must not use it inside the loop.

Your code could look like:

void vectorremove(std::vector<std::string>& v) {
    auto end{ v.end() };
    for (auto it = v.begin(); it != end; ++it)
    {
        end = std::remove(it + 1, end, *it);
    }
    v.erase(end, v.end());
}

Note the usage of auto instead of std::vector<std::string>::iterator.

However, STL provides handy functions to achieve what you want. One of them is std::unique which

Eliminates all but the first element from every consecutive group of equivalent elements from the range [first, last) and returns a past-the-end iterator for the new logical end of the range.

In order to remove duplicates from the std::vector, you can do something like:

#include <iostream>
#include <algorithm>
#include <vector>

int main()  {
    std::vector<int> v{ 1, 2, 3, 1, 2, 3, 3, 4, 5, 4, 5, 6, 7 };
    std::sort(v.begin(), v.end()); // 1 1 2 2 3 3 3 4 4 5 5 6 7 
    auto last = std::unique(v.begin(), v.end());
    v.erase(last, v.end());
    for (auto const i : v) {
        std::cout << i << " ";
    }
    std::cout << std::endl;

    return 0;
}

Remember that std::unique works as expected only on sorted std::vectors.

NutCracker
  • 11,485
  • 4
  • 44
  • 68
0

You only modify the copy of vector , you have to pass by reference to modify the actual vector, why you don't use auto instead of std::vector::iterator. and you have to know that erase invalidates all iterators pointing to the erased element and beyond the erased element, keep the iterator valid by using erase's return value, also use std::getline inside the loop to store the value from std::cin to include the new line.

Alternatively your can use std::unique removes the duplicate value and works as expected after sorting the elements. and std::unique returns a past the end iterator for the new logical end of the range:-

#include <vector>
#include <algorithm>
#include <string>

std::vector<std::string> removeDuplicate(std::vector<std::string> & v){

std::vector<std::string> vec;

std::sort(std::begin(v), std::end(v));
auto pos = std::unique(std::begin(v), std::end(v));

vec.assign(std::begin(v), pos);
return vec;
}


int main(){

std::vector<std::string> vect{"John", "John", "Paul", "John", "Lucy", "Bob", "Bob"};

auto pureVector = removeDuplicate(vect);

for(auto const & v : pureVector){
    std::cout << v << '\n';
}

}
Owl66
  • 147
  • 12