0

I've written a code that removes all vowels from a string in c++ but for some reason it doesn't remove the vowel 'o' for one particular input which is: zjuotps.

Here's the code:

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

int main(){
    string s;
    cin >> s;

    string a = "aeiouyAEIOUY";

    for (int i = 0; i < s.length(); i++){
        for(int j = 0; j < a.length(); j++){
            if(s[i] == a[j]){
                s.erase(s.begin() + i);
            }
        }
    }
    cout << s;

    return 0;
}

When I input: zjuotps

The Output I get is: zjotps

Gabriel Staples
  • 36,492
  • 15
  • 194
  • 265
NerdNet
  • 359
  • 1
  • 3
  • 11
  • 4
    Most inputs with two vowels in a row will fail. When this code finds a vowel, it is removed from the string **and** it keeps searching the same index for later vowels **and** it increments `i` – Drew Dormann Jan 14 '22 at 15:26
  • oh. Why is that? – NerdNet Jan 14 '22 at 15:27
  • 3
    @NerdNet, Because after you remove that vowel the next letter's index is one less and you skip over it – asimes Jan 14 '22 at 15:28
  • @DrewDormann I tried inputting something like "aeiou" and my output was " ". So maybe it isn't about vowels in a row? – NerdNet Jan 14 '22 at 15:29
  • Use your debugger to figure out what your code is doing instead of guessing or asking us to debug for you. – drescherjm Jan 14 '22 at 15:30
  • 2
    Use your debugger. Or use a pencil an paper. Imagine the string `"uo"` and see if you can figure out why this code will delete the `u`, but not delete the `o`. – Drew Dormann Jan 14 '22 at 15:31
  • 2
    You have to be pretty careful when modifying the structure of an underlying container - if I'm not mistaken the iterators will end up getting updated on each deletion. If you must use `string.erase(),` something that might work is having a separate variable for `numberOfDeletions` which is incremented after each erase. From there, the correct call to erase would be `s.erase(s.begin() + i - numberOfDeletions)`. A far better algorithm would be to just copy the non-vowels into a separate string, however. – wLui155 Jan 14 '22 at 15:32
  • 1
    If a shorter explanation would be more helpful - what this code does _immediately_ after erasing a character is clearly wrong. – Drew Dormann Jan 14 '22 at 15:44

4 Answers4

5

This is a cleaner approach using the C++ standard library:

#include <algorithm>
#include <iostream>
#include <string>

using namespace std;

int main()
{
    std::string input = "zjuotps";
    std::string vowels = "aeiouyAEIOUY";

    auto predicate = [&vowels](char c) { return vowels.find(c) != std::string::npos; };
    auto iterator = std::remove_if(input.begin(), input.end(), predicate);
    input.erase(iterator, input.end());

    cout << input << endl;
}

Edit:

as @RemyLebeau pointed out, std::erase_if can be used which is introduced in c++20 and the answer becomes one line of code:

std::erase_if(input, [&vowels](char c) { return vowels.find(c) != std::string::npos; });
Shahriar
  • 768
  • 4
  • 11
  • If you're trying to use similar variable names as in the question, `a` is the vowel string and `s` is the input string. However, for readability, I strongly recommend updating your answer to use something more-readable instead, like `input_str` as the input string, and `vowels` as the vowel string. – Gabriel Staples Jan 14 '22 at 19:09
  • 1
    Thanks @GabrielStaples I fixed it. – Shahriar Jan 14 '22 at 19:17
  • 2
    In C++20, you can use [`std::erase_if()`](https://en.cppreference.com/w/cpp/string/basic_string/erase2), eg: `std::erase_if(input, predicate);` – Remy Lebeau Jan 15 '22 at 07:11
1

You can develop a solution by adding the matching characters to the new string object. The eliminate() method writes the character to the result object if the characters in the input object doesn't match the characters in the remove object.

#include <iostream>

/**
 * @brief This method scans the characters in the "input" object and writes
 *        the characters not in the "remove" object to the "result" object.
 * @param input This object contains the characters to be scanned.
 * @param remove This object contains characters that will not match.
 * @param result Non-match result data is writed to this object.
 */
void eliminate(std::string input, std::string remove, std::string &result);

int main()
{
    std::string input = "zjuotpsUK", remove = "aeiouyAEIOUY", result;
    eliminate(input, remove, result);
    std::cout << result << std::endl;
    return 0;
}

void eliminate(std::string input, std::string remove, std::string &result)
{
    for (size_t i = 0, j = 0; i < input.length(); i++)
    {
        for(j = 0; j < remove.length(); j++)
            if(input[i] == remove[j])
                break;

        if(j == remove.length())
            result += input[i];
    }
}
Sercan
  • 4,739
  • 3
  • 17
  • 36
1

In your code here, I replaced s with input_str, and a with vowels, for readability:

for (int i = 0; i < input_str.length(); i++){
    for(int j = 0; j < vowels.length(); j++){
        if(input_str[i] == vowels[j]){
            input_str.erase(input_str.begin() + i);
        }
    }
}

The problem with your current code above is that each time you erase a char in the input string, you should break out of the vowels j loop and start over again in the input string at the same i location, checking all vowels in the j loop again. This is because erasing a char left-shifts all chars which are located to the right, meaning that the same i location would now contain a new char to check since it just left-shifted into that position from one position to the right. Erroneously allowing i to increment means you skip that new char to check in that same i position, thereby leaving the 2nd vowel in the string if 2 vowels are in a row, for instance. Here is the fix to your immediate code from the question:

int i = 0;
while (i < s.length()){
    bool char_is_a_vowel = false;
    for(int j = 0; j < a.length(); j++){
        if(s[i] == a[j]){
            char_is_a_vowel = true;
            break; // exit j loop
        }
    }
    if (char_is_a_vowel){
        s.erase(s.begin() + i);
        continue; // Do NOT increment i below! Skip that.
    }
    i++;
}

However, there are many other, better ways to do this. I'll present some below. I personally find this most-upvoted code difficult to read, however. It requires extra study and looking up stuff to do something so simple. So, I'll show some alternative approaches to that answer.

Approach 1 of many: copy non-vowel chars to new string:

So, here is an alternative, simple, more-readable approach where you simply scan through all chars in the input string, check to see if the char is in the vowels string, and if it is not, you copy it to an output string since it is not a vowel:

Just the algorithm:

std::string output_str;
for (const char c : input_str) {
    if (vowels.find(c) == std::string::npos) {
        output_str.push_back(c);
    }
}

Full, runnable example:

#include <iostream>  // For `std::cin`, `std::cout`, `std::endl`, etc.
#include <string>

int main()
{
    std::string input_str = "zjuotps";
    std::string vowels = "aeiouyAEIOUY";

    std::string output_str;
    for (const char c : input_str)
    {
        if (vowels.find(c) == std::string::npos)
        {
            // char `c` is NOT in the `vowels` string, so append it to the 
            // output string
            output_str.push_back(c);
        }
    }

    std::cout << "input_str  = " << input_str << std::endl;
    std::cout << "output_str = " << output_str << std::endl;
}

Output:

input_str  = zjuotps
output_str = zjtps

Approach 2 of many: remove vowel chars in input string:

Alternatively, you could remove the vowel chars in-place as you originally tried to do. But, you must NOT increment the index, i, for the input string if the char is erased since erasing the vowel char left-shifs the remaining chars in the string, meaning that we need to check the same index location again the next iteration in order to read the next char. See the note in the comments below.

Just the algorithm:

size_t i = 0;
while (i < input_str.length()) {
    char c = input_str[i];
    if (vowels.find(c) != std::string::npos) {
        input_str.erase(input_str.begin() + i);
        continue;
    }
    i++;
}

Full, runnable example:

#include <iostream>  // For `std::cin`, `std::cout`, `std::endl`, etc.
#include <string>

int main()
{
    std::string input_str = "zjuotps";
    std::string vowels = "aeiouyAEIOUY";

    std::cout << "BEFORE: input_str = " << input_str << std::endl;

    size_t i = 0;
    while (i < input_str.length())
    {
        char c = input_str[i];
        if (vowels.find(c) != std::string::npos)
        {
            // char `c` IS in the `vowels` string, so remove it from the
            // `input_str`
            input_str.erase(input_str.begin() + i);
            // do NOT increment `i` here since erasing the vowel char above just
            // left-shifted the remaining chars in the string, meaning that we
            // need to check the *same* index location again the next
            // iteration!
            continue;
        }
        i++;
    }

    std::cout << "AFTER:  input_str = " << input_str << std::endl;
}

Output:

BEFORE: input_str = zjuotps
AFTER:  input_str = zjtps

Approach 3 of many: high-speed C-style arrays: modify input string in-place

I borrowed this approach from "Approach 1" of my previous answer here: Removing elements from array in C

If you are ever in a situation where you need high-speed, I'd bet this is probably one of the fastest approaches. It uses C-style strings (char arrays). It scans through the input string, detecting any vowels. If it sees a char that is NOT a vowel, it copies it into the far left of the input string, thereby modifying the string in-place, filtering out all vowels. When done, it null-terminates the input string in the new location. In case you need a C++ std::string type in the end, I create one from the C-string when done.

Just the algorithm:

size_t i_write = 0;
for (size_t i_read = 0; i_read < ARRAY_LEN(input_str); i_read++) {
    bool char_is_a_vowel = false;
    for (size_t j = 0; j < ARRAY_LEN(input_str); j++) {
        if (input_str[i_read] == vowels[j]) {
            char_is_a_vowel = true;
            break;
        }
    }
    if (!char_is_a_vowel) {
        input_str[i_write] = input_str[i_read];
        i_write++;
    }
}
input_str[i_write] = '\n';

Full, runnable example:

#include <iostream>  // For `std::cin`, `std::cout`, `std::endl`, etc.
#include <string>

/// Get the number of elements in an array
#define ARRAY_LEN(array) (sizeof(array)/sizeof(array[0]))

int main()
{
    char input_str[] = "zjuotps";
    char vowels[] = "aeiouyAEIOUY";

    std::cout << "BEFORE: input_str = " << input_str << std::endl;

    // Iterate over all chars in the input string
    size_t i_write = 0;
    for (size_t i_read = 0; i_read < ARRAY_LEN(input_str); i_read++)
    {
        // Iterate over all chars in the vowels string. Only retain in the input
        // string (copying chars into the left side of the input string) all
        // chars which are NOT vowels!
        bool char_is_a_vowel = false;
        for (size_t j = 0; j < ARRAY_LEN(input_str); j++)
        {
            if (input_str[i_read] == vowels[j])
            {
                char_is_a_vowel = true;
                break;
            }
        }
        if (!char_is_a_vowel)
        {
            input_str[i_write] = input_str[i_read];
            i_write++;
        }
    }
    // null-terminate the input string at its new end location; the number of
    // chars in it (its new length) is now equal to `i_write`!
    input_str[i_write] = '\n';

    std::cout << "AFTER:  input_str = " << input_str << std::endl;
    // Just in case you need it back in this form now:
    std::string str(input_str);
    std::cout << "          C++ str = " << str << std::endl;
}

Output:

BEFORE: input_str = zjuotps
AFTER:  input_str = zjtps
          C++ str = zjtps

See also:

  1. [a similar answer of mine in C] Removing elements from array in C
Gabriel Staples
  • 36,492
  • 15
  • 194
  • 265
  • Your #2 seems odd to me. I'm used to seeing this implementation, which is shorter, and also works on linked lists. http://coliru.stacked-crooked.com/a/6f8dd8002977c908 – Mooing Duck Jun 21 '23 at 15:51
0

Try this easy approach.

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

int main()
{
    string L;
    
    std::cout << "Enter the string" << std::endl;
    std::cin >> L;
    for(int i=0;L[i];i++){
        if(L[i]=='i' || L[i]=='a' || L[i]=='e' || L[i]=='o' || L[i]=='u' || L[i]=='A' || L[i]=='E' || L[i]=='I' || L[i]=='O' || L[i]=='U'){
            L[i]='\0'; //replacing with null
        }
     
    }
    std::cout << L << std::endl;
    return 0;
}

I can see that this is a little bit confusing, bare with me. A null value is used to terminate a string. For a regular c-style string null value means 'the end' but a C++ string is an instance of the class std::string that is part of the C++ standard library. Std::string can contain null characters.

So std::string can contain embedded null characters within it, but a C- string can not do that as it ends at the first null character.

  • 1
    Although your code works, it would be nice to have an explanation as to *how* it does. For example, many readers might think that adding a `nul` character would terminate the string at that point. Also, you never use your `j` variable and the `else ... continue` block seems redundant. – Adrian Mole May 09 '23 at 17:05
  • 1
    @AdrianMole I hope my edited message helps. Ah yes, I forgot to remove those unnecessary stuffs as I was testing something else at that time. Happy coding! – Levin Mcleish Jun 21 '23 at 16:46