2

I am learning C++. Today I have written a code to remove vowels form a string. It works fine in some tests. But this test is failing to remove "u" from a string. My input was: tour. Output was: tur. But I am expecting the output like tr for tour

Code:

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

int main()
{
    string word;
    getline(cin, word);
    transform(word.begin(), word.end(), word.begin(), ::tolower);  // Converting uppercase to lowercase
    for (int i = 0; i < word.length(); i++)
    {
        if (word[i] == 'a' || word[i] == 'e' || word[i] == 'i' || word[i] == 'o' || word[i] == 'u')
        {
            word.erase(word.begin() + i);  // Removing specific character
        }
    }
    cout << word << endl;

    return 0;
}

How can I do that? Where is the problem in the code?

cigien
  • 57,834
  • 11
  • 73
  • 112
  • I confirmed the behavior here: [https://ideone.com/o6wTOs](https://ideone.com/o6wTOs) – drescherjm Jul 16 '20 at 15:00
  • 3
    Suggested reading: [Why is "using namespace std" considered bad practice](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) and [Why should I not #include bits stdc++](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) – ChrisMM Jul 16 '20 at 15:02

3 Answers3

3

When you erase a character, your index i is still being incremented, which means you are skipping checking characters that appear right after a vowel.

You need to decrement i when you erase a character:

if (word[i] == 'a' || word[i] == 'e' || word[i] == 'i' || word[i] == 'o' || word[i] == 'u')
{
    word.erase(word.begin() + i);  // Removing specific character
    --i;  // make sure to check the next character
}

Here's a demo.

Also, please avoid the following 2 lines:

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

They are dangerous and should not be used.

cigien
  • 57,834
  • 11
  • 73
  • 112
2

After erase, i++ is performed. That means one element is bypassed by the check.

Change the for loop to

for (int i = 0; i < word.length(); )
{
    if (word[i] == 'a' || word[i] == 'e' || word[i] == 'i' || word[i] == 'o' || word[i] == 'u')
    {
        word.erase(word.begin() + i);  // Removing specific character
    } 
    else 
    {
        i++; // increment when no erasion is performed
    }
}
songyuanyao
  • 169,198
  • 16
  • 310
  • 405
  • Thank you so much sir.The problem is now solved and I understand your answer very easily. –  Jul 16 '20 at 15:18
2

Here's how you can do it more easily:

#include <algorithm>
#include <array>
#include <cctype>
#include <iostream>
#include <string>

constexpr auto isVowel(unsigned char const ch) noexcept {
  constexpr std::array<unsigned char, 10> vowels{
      'a', 'e', 'i', 'o', 'u',
  };
  return std::find(vowels.begin(), vowels.end(), std::tolower(ch)) !=
         vowels.end();
}

int main() {
  std::string str = "Tour";
  str.erase(std::remove_if(str.begin(), str.end(), isVowel), str.end());
  std::cout << str << '\n';
}

Try online

Aykhan Hagverdili
  • 28,141
  • 6
  • 41
  • 93
  • 1
    g++ was not compiling with std::find until I enabled c++20: [https://godbolt.org/z/WzWEoh](https://godbolt.org/z/WzWEoh) – drescherjm Jul 16 '20 at 15:16
  • 1
    @drescherjm good point. Now that `std::find` is `constexpr` it should not be a problem. And you can always remove the `constexpr` as it's not necessary. – Aykhan Hagverdili Jul 16 '20 at 15:24