yak
yak

Reputation: 3930

How to remove from vector string with letters only? My code does not remove every string I need to delete

In my application I have a vector of strings. In this vector I have various strings, and would like to remove from my vector strings that contain only letters. I wrote something like this:

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

bool lettersOnly(std::string text)
{
    for(int i=0; i<text.length(); i++)
    {
        if(!isalpha(text.at(i)))
            return false;
    }
    return true;
}

void removeWordsWithLettersOnly(std::vector<std::string> &vec)
{
    for(int i=0; i<vec.size(); i++)
    {
        std::string text = vec.at(i);
        if(lettersOnly(text))
            vec.erase(remove(vec.begin(), vec.end(), text), vec.end());
    }
}

int main()
{
    vector<string> vec;

    vec.push_back("Someletters");
    vec.push_back("Bomeletters");
    vec.push_back("someletters");
    vec.push_back("123456543");
    vec.push_back("098765");
    vec.push_back("someletters");
    vec.push_back("someletters");
    vec.push_back("someletters234567");

    for(int i=0; i<vec.size(); i++)
        cout << vec[i] << "\n";

    cout << "\n\n\n";
    removeWordsWithLettersOnly(vec);

    for(int i=0; i<vec.size(); i++)
        cout << vec[i] << "\n";

    return 0;

}

The problem is, that ever I use my method on my vector, it does not remove every string it should. For my example code above, it does not remove the string Bomeletters. Why is that? This string has letters only, so should be removed to. Any ideas?

Upvotes: 0

Views: 408

Answers (2)

juanchopanza
juanchopanza

Reputation: 227390

Your loop is complicating things by removing elements from under your feet.

You can simply use std::remove_if with your predicate like this:

void removeWordsWithLettersOnly(std::vector<std::string> &vec)
{
  vec.erase(remove_if(vec.begin(), vec.end(), lettersOnly), vec.end());
}

With that in place, your code works as expected.

Note that your lettersOnly predicate is making unnecessary copies of each string passed to it. You can fix this by making its parameter a const reference instead:

bool lettersOnly(const std::string& text) { /* as before */ }

Upvotes: 4

nvelan
nvelan

Reputation: 96

You are removing from the vector while iterating it.

for example: When "Someletters" is deleted -> your vector is decreased in size and now "Bomeletters" is in position 0. However your counter i is already at position 1 -> so "Bomeletters" is missed.

The simplest way to solve this is to just copy the non filtered strings to a new vector, then use "swap()" to replace the content of the 2 vectors.

Upvotes: 2

Related Questions