CODEWITHSUNDEEP
c++
scypx
scypx

Reputation: 87

Counting vowels c++

While user inputs 10 alphabets, program should tell how many of them are vowels. I have written this code:

while (count<10)
    {
        cin >> n;
        count++;
        if (n == 'A' || n == 'a' && n == 'E' || n == 'e' && n == 'I' || n == 'i' && n == 'O' || n == 'o' && n == 'U' || n == 'u')
            {
                total++;
            }
    }

cout <<  total << endl;

This generates output of 0 even if there are vowels entered by user. something wrong?

Upvotes: 3

Views: 2068

Answers (2)

Kostas
Kostas

Reputation: 4176

I recommend using a switch statement with fall-through, which is more readable but also potentially more efficient (may be implemented with jump table).

int count = 10;
while (count--) {
    switch(std::tolower(n)) {
        case 'a': case 'e': 
        case 'i': case 'o': 
        case 'u': total ++; break;
        default:;
    }
}

Upvotes: 2

Ryan Haining
Ryan Haining

Reputation: 36862

Let's start by cutting down the condition down a bit, to only look at a and e

if (n == 'A' || n == 'a' && n == 'E' || n == 'e')

and then only considering the lowercase letters for simplicity (but retains the problem)

if (n == 'a' && n == 'e')

if you read this out loud it says "if n is 'a' AND n is 'e'". The "AND" (from the && operator) means that both conditions must be true. You've created an impossible condition here, if n is 'a', then it is not 'e', so you get if (true && false) - which is false. If n is 'e', then it is not 'a', so you get if (false && true).

Simply replace all of your && (and) operators with || (or) operators to have the condition be true if at least one of your equality comparisons is true.

if (n == 'A' || n == 'a' || n == 'E' || n == 'e' 
    || n == 'I' || n == 'i' || n == 'O' || n == 'o' 
    || n == 'U' || n == 'u')

There are some ways to simplify the condition.

One is to add a #include <cctype> and use std::tolower to convert n to lowercase, then you only need to compare against lowercase characters.

n = std::tolower(n);
if (n == 'a' || n == 'e' || n == 'i' || n == 'o' || n == 'u')

Another, less repetitive approach is to create a std::string with all the vowels in it, and then see if it contains n, #include <string> up top.

std::string vowels = "aeiouAEIOU";
while (/*...*/) {
   // ...
   if (vowels.find(n) != std::string::npos) {
     ++total;
   }
}

As n314159 pointed out, if you are using C++17 or later, you can use a std::string_view instead, which is cheaper. #include <string_view>

static constexpr std::string_view vowels = "aeiouAEIOU";   
while (/*...*/) {
   // ...
   if (vowels.find(n) != std::string_view::npos) {
     ++total;
   }
}

Upvotes: 7

Related Questions