0

I wrote this code to remove consonants from string but my answer is not correct. So what change should i make in this code to make it work correctly. Further more when i remove the space character i.e. last element from unordered set then the output was unexpected to me. So please help me understanding what is going on in this code??

#include<bits/stdc++.h>
using namespace std;

int main() {
   unordered_set<char> a={'a','e','i','o','O','u','U','A','E','I',' '};

   string s="what are you doing";

   string ::iterator it;
   for(it=s.begin();it!=s.end();it++){
      if(a.find(*it)==a.end()){
         s.erase(it);}
    }   

    cout<<s;
    return 0;
}
JeJo
  • 30,635
  • 6
  • 49
  • 88
Himanshu
  • 35
  • 5
  • 1
    You are changing the string while looping, that doesn't work since you invalidate your iterator. – πάντα ῥεῖ Aug 17 '18 at 15:45
  • 1
    @πάνταῥεῖ actually `std::string::erase` doc does not say it invalidates iterators https://en.cppreference.com/w/cpp/string/basic_string/erase hmm – Slava Aug 17 '18 at 16:00
  • Recommendation: Use caution with `#include` ([explanation](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h)) and `using namespace std;` ([explanation](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice)). Use greater caution when you use both because they magnify some of each other's worst effects. – user4581301 Aug 17 '18 at 16:06
  • 1
    Why downvote and close request? OP created [mcve] and provided enough information IMHO. – Slava Aug 17 '18 at 16:23

1 Answers1

2

Your code has issue, when you remove symbol

s.erase(it);

and later increase it by loop you skip a symbol and also may go over the end of string. Possible solution to fix your code:

for(it=s.begin();it!=s.end();){
   if(a.find(*it)==a.end())
      it = s.erase(it);
   else
      ++it;
 }

Note1: usually in STL containers when erasing element by iterator invalidate that iterator. Though documentation for std::string::erase() does not say that any iterator invalidated and possibly it is not the case here. Anyway code I provided will work in any case and with any container and solves your issue as well.

But better to use erase-remove idiom using std::remove_if():

   s.erase( std::remove_if( s.begin(), s.end(), [a] ( char c ){ return a.count(c) == 0; } ), s.end() );

Note2: it is not a good idea to include "bits/stdc++.h"

live example

Slava
  • 43,454
  • 1
  • 47
  • 90
  • must be a failure in cppreference, for example an `end()` iterator is certainly invalidated.... – 463035818_is_not_an_ai Aug 17 '18 at 16:17
  • @user463035818 right, still not clear if `it` is invalidated. Looks like deserves another question in SO. – Slava Aug 17 '18 at 16:21
  • The remove/erase-idiom has linear worst-case complexity. For the other approach it is quadratic. Thus, it is ill-advised to use it and the discussion on whether iterators get invalidated is kind of moot. Note, that the end of the string changes. As strings are contiguous a copy of the former iterator points beyond the end of the range and is clearly invalid. – Dietmar Kühl Aug 17 '18 at 16:37