Return last vowel in string

Viewed 213

I need to return the last vowel (uppercase or lowercase) in a given string and return '!' if there aren't any vowels in the string.

The code I wrote works but only if the string ends with a vowel. If it doesn't end with a vowel it always returns a '!'.

How can I fix this?

char lastVowel(string s){

    char vowel = ' ';

    for(int i = 0; i < s.size(); ++i){
        if(s[i] == 'A' || s[i] == 'E' || s[i] == 'I' || s[i] == 'O' || s[i] == 'U' ||
           s[i] == 'a' || s[i] == 'e' || s[i] == 'i' || s[i] == 'o' || s[i] == 'u'){
            vowel = s[i];
        }else{
            vowel = '!';
        }
    }
    return vowel;
}
2 Answers

This is really simple and can be done in two lines:

#include <string>

char lastVowel(std::string s) {
    const auto pos = s.find_last_of("aeiouAEIOU");
    return pos == std::string::npos ? '!' : s[pos];
}

You could also std::transform the string into lowercase and shorten the search to just aeiou, but I haven't tested whether or not this impacts performance.

EDIT:

I just tested it. For large N (100,000 character string) using std::transform is about 158 times slower.

Do not convert your string to one case or the other first. It is MUCH faster to just make the search criteria longer.

You set vowel to ! every time you find a non-vowel character in your string. Instead, you need to simply do this:

char lastVowel(string s){
    // Default value is '!'
    char vowel = '!';

    for(int i = 0; i < s.size(); ++i){
        if(s[i] == 'A' || s[i] == 'E' || s[i] == 'I' || s[i] == 'O' || s[i] == 'U' ||
           s[i] == 'a' || s[i] == 'e' || s[i] == 'i' || s[i] == 'o' || s[i] == 'u'){
            vowel = s[i]; // Only set vowel when a vowel character is found
        }
    }
    // If a vowel was set in the loop, you return that
    // Otherwise, you return its default value (!)
    return vowel;
}

While this answers why you encounter that issue, and how to fix it with minimal changes to your approach, Casey's approach below is better because it uses standard library functions that look for the characters of interest from the end of the string -- like other commenters have said, if you care about the last occurrence of something, it's smarter to start looking in the end.

Related