Return to Answer

bool is_vowel(char ch) {
    auto lowercase_ch = ch | 0x20;
    if (static_cast<unsigned char>(lowercase_ch - 96) > 31) {
       return false;
    }
    constexpr std::uint32_t vowel_mask = (1 << 1) | (1 << 5) | (1 << 9) | (1 << 15) | (1 << 21);
    return (vowel_mask & (std::uint32_t{1} << lowercase_ch)) != 0;
}

bool is_vowel(char ch) {
    auto lowercase_pos = (ch | 0x20) - 96;
    if (static_cast<unsigned char>(lowercase_pos) > 31) {
       return false;
    }
    constexpr std::uint32_t vowel_mask = (1 << 1) | (1 << 5) | (1 << 9) | (1 << 15) | (1 << 21);
    return (vowel_mask & (std::uint32_t{1} << lowercase_pos)) != 0;
}

Don't define a variable before it is needed. Doing so increases the cognitive load on readers who have to trace through your code and keep a mental record of all of the variables, their possible values, whether they've even been set. It also encourages unintended reuse. There are performance exceptions (allocated buffer reuse), but none in your program. Define the variables as you need them and keep the scope of each variable as narrow as required.

Try to use more descriptive names. The larger the scope in which a variable will live, the more important it is to give it a name that indicates to the reader what data it represents. String \$\rightarrow\$ input_sentence. Nvowels \$\rightarrow\$ vowel_count.

Whitespaces are great for readability, as they help separate operators and language constructs.

Avoid std::endl. If you need to explicitly flush the cache, the explicitly stream std::flush and comment why a flush is necessary. If you simply need a end-of-line character, prefer '\n' or "\n".

As with std::istream::getline, std::istream::operator>> has certain postconditions that need to be checked to ensure a read was successful. If extraction failed, you should probably notify the user and attempt to recover if necessary.

Keep comments crisp. When reading a piece of code, everything I need to know as far as what is being done is already there in the form of code to be compiled. Comments should be reserved for explaining why the code was written the way it was (the intent). Adding verbosity slows down understanding.

Using std::fflush on an input stream is undefined behavior. If you were trying to reset the buffer of std::cin, see std::cin::ignore.

Once the user selection option \$E\$ and the program encounters exit(0), all of the remaining code (break; \$\rightarrow\$ while (choice != 'E') \$\rightarrow\$ system("pause");) is unreachable. If you intend to immediately return from a function (or in this case exit the program), you don't need to loop until choice is \$E\$. This is one of those times where while (true) is actually appropriate if you didn't have the restriction.

Keep in mind the various possible values a type can represent. A character (char) can represent the set of consonants and vowels (A-Z, a-z). It can also represent digits, punctuation, control characters, whitespace, etc. The conditional in Consonant_count is returning true or false if the current character is any ASCII character (not any alphabetic character) is not any of aeiou. Clearly a bug?

Learn the <algorithm>s. You'll notice that both your functions have the same structure as the pseudocode above. It turns out, it's a common algorithm and in C++, it's called std::count_if. You should be aware of this library as you learn more about functions and specifically lambdas (objects that behave like functions and can be passed around into functions as arguments).

Don't define a variable before it is needed. Doing so increases the cognitive load on readers that trace through your code and keep mental records of every variable, their possible values, and whether they've even been set. This artifact from older languages also encourages lazy reuse of variables beyond their original intent. Define variables as you need them and keep the scope of each variable as narrow as required.

Use descriptive names. The larger the scope in which a variable exists, the more important it is to give that variable a name that indicates to the reader what it represents. String \$\rightarrow\$ input_sentence. Nvowels \$\rightarrow\$ vowel_count.

Whitespace is great for readability as they help separate operators and language constructs.

Avoid std::endl. If you need to explicitly flush the cache, then explicitly stream the manipulator std::flush and comment why a flush is necessary. If you simply need a end-of-line character, prefer '\n' or "\n".

As with std::istream::getline, std::istream::operator>> needs to be checked to ensure a read was successful. If extraction failed, you should probably notify the user and attempt to recover.

Keep comments crisp. Everything I need to know as far as what is being done is already there in the form of code to be compiled. Comments should be reserved for explaining why the code was written the way it was (the intent). Adding verbosity slows down understanding.

Using std::fflush on an input stream is undefined behavior. If you were trying to reset the buffer of std::cin, see std::cin::ignore.

Once the the program encounters exit(0), all of the remaining code (break; \$\rightarrow\$ while (choice != 'E') \$\rightarrow\$ system("pause");) is unreachable. If you intend to immediately return from a function (or in this case exit the program), you don't need to loop until choice is \$E\$. This is one of those times where while (true) is actually appropriate if you didn't have the restriction.

Keep in mind the various possible values a type can represent. A character (char) can represent the set of consonants and vowels (A-Z, a-z). It can also represent digits, punctuation, control characters, whitespace, etc. The conditional in Consonant_count is returning true or false if the current character is any ASCII character (not any alphabetic character) that is not any of aeiou. Clearly control structures, punctuation, and whitespaces are not consonants.

bool is_vowel(char ch) {
    auto lowercase_ch = ch | 0x20;
    if (static_cast<unsigned char>(lowercase_ch - 96) > 31) {
       return false;
    }
    constexpr std::uint32_t vowel_mask = (1 << 1) | (1 << 5) | (1 << 9) | (1 << 15) | (1 << 21);
    return (vowel_mask & (std::uint32_t{1} << lowercase_ch)) != 0;
}

Learn the various <algorithm>s. You'll notice that both your functions have the same structure as the pseudocode above. It turns out, this is a common algorithm in C++ (std::count_if). You should be aware of this library as you learn more about functions and specifically lambdas (objects that behave like functions and can be passed around into functions as arguments).

Don't define a variable before it is needed. Doing so reduces the cognitive load on readers who have to trace through your code and keep a mental record of all of the variables, their possible values, whether they've even been set. It also encourages unintended reuse. There are performance exceptions (allocated buffer reuse), but none in your program. Define the variables as you need them and keep the scope of each variable as narrow as required.

Don't define a variable before it is needed. Doing so increases the cognitive load on readers who have to trace through your code and keep a mental record of all of the variables, their possible values, whether they've even been set. It also encourages unintended reuse. There are performance exceptions (allocated buffer reuse), but none in your program. Define the variables as you need them and keep the scope of each variable as narrow as required.