Program is counting consonants wrong

Question

I'm trying to make a program that counts all the vowels and all the consonants in a text file. However, if the file has a word such as cat it says that there are 3 consonants and 1 vowel when there should be 2 consonants and 1 vowel.

#include <string>
#include <cassert>
#include <cstdio>

using namespace std;

int main(void)
{
    int i, j;
    string inputFileName;

    ifstream fileIn;
    char ch;
    cout<<"Enter the name of the file of characters: ";
    cin>>inputFileName;
    fileIn.open(inputFileName.data());
    assert(fileIn.is_open());
    i=0;
    j=0;



    while(!(fileIn.eof())){
        ch=fileIn.get();

        if (ch == 'a'||ch == 'e'||ch == 'i'||ch == 'o'||ch == 'u'||ch == 'y'){
            i++;
        }

        else{
            j++;
        }
    }

    cout<<"The number of Consonants is: " << j << endl;
    cout<<"The number of Vowels is: " << i << endl;

    return 0;
}

Answer 1

Your program doesn't check for numbers and special characters, as well as uppercase letters. Plus, the .eof() is misused: it gets to the last character of the file, loops again, reads one more character, and only then it realizes it is at the end of the file, generating the extra consonant problem. Consider using while((ch = inFile.get()) != EOF).

Answer 2

Here you check if the eof state is set, then try to read a char. eof will not be set until you try to read beyond the end of the file, so reading a char fails, but you'll still count that char:

while(!(fileIn.eof())){
        ch=fileIn.get();   // this fails and sets eof when you're at eof

So, if your file only contains 3 chars, c, a and t and you've read the t you'll find that eof() is not set. It'll be set when you try reading the next char.

A better way is to check if fileIn is still in a good state after the extraction:

while(fileIn >> ch) {

With that in place the counting should add up. All special characters will be counted as consonants though. To improve on that, you could check that the char is a letter:

#include <cctype>

// ...

    while(fileIn >> ch) {
        if(std::isalpha(ch)) {        // only count letters
            ch = std::tolower(ch);    // makes it possible to count uppercase letters too
            if(ch == 'a' || ch == 'e' || ch == 'i' || ch == 'o' || ch == 'u' || ch == 'y') {
                i++;
            } else {
                j++;
            }
        }
    }

Answer 3

I first thought my very first comment to your question was just a sidenote, but in fact it's the reason for the results you're getting. Your reading loop

while(!(fileIn.eof())){
    ch=fileIn.get();
    // process ch
}

is flawed. At the end of the file you'll check for EOF with !fileIn.eof() but you haven't read past the end yet so your program enters the loop once again and fileIn.get() will return EOF which will be counted as a consonant. The correct way to read is

while ((ch = file.get()) != EOF) {
    // process ch
}

with ch declared as integer or

while (file >> ch) {
    // process ch 
}

with ch declared as char. To limit the scope of ch to the loop consider using a for-loop:

for (int ch{ file.get() }; ch != EOF; ch = file.get()) {
    // process ch;
}

---

As @TedLyngmo pointed out in the comments, EOF could be replaced by std::char_traits<char>::eof() for consistency although it is specified to return EOF.

---

Also your program should handle everything that isn't a letter (numbers, signs, control characters, ...) differently from vowels and consonants. Have a look at the functions in <cctype>.

Answer 4

I would use a different approach, searching strings:

const std::string vowels = "aeiou";
int vowel_quantity = 0;
int consonant_quantity = 0;
char c;
while (file >> c)
{
    if (isalpha(c))
    {
        if (vowels.find(c) != std::string::npos)
        {
            ++vowel_quantity;
        }
        else
        {
            ++consonant_quantity;
        }
    }
}

Note: in the above code fragment, the character is first tested for an alphabetic characters. Characters may not be alphabetical like period or question mark. Your code counts periods as consonants.

Edit 1: character arrays
If you are not allowed to use std::string, you could also use character arrays (a.k.a. C-Strings):

static const char vowels[] = "aeiou";  
int vowel_quantity = 0;
int consonant_quantity = 0;
char c;
while (file >> c)
{
    if (isalpha(c))
    {
        if (strchr(vowels, c) != NULL)
        {
            ++vowel_quantity;
        }
        else
        {
            ++consonant_quantity;
        }
    }
}

Answer 5

In addition to Why !.eof() inside a loop condition is always wrong., you have another test or two you must implement to count all vowels and consonants. As mentioned in the comment, you will want to use tolower() (by including cctype) to convert each char to lower before your if statement to ensure you classify both upper and lower-case vowels.

In addition to testing for vowels, you need an else if (isalpha(c)) test. You don't want to classify whitespace or punctuation as consonants.

Additionally, unless you were told to treat 'y' as a vowel, it technically isn't one. I'll leave that up to you.

Adding the tests, you could write a short implementation as:

#include <iostream>
#include <fstream>
#include <string>
#include <cctype>

int main (void) {

    size_t cons = 0, vowels = 0;
    std::string ifname {};
    std::ifstream fin;

    std::cout << "enter filename: ";
    if (!(std::cin >> ifname)) {
        std::cerr << "(user canceled input)\n";
        exit (EXIT_FAILURE);
    }

    fin.open (ifname);
    if (!fin.is_open()) {
        std::cerr << "error: file open failed '" << ifname << "'\n";
        exit (EXIT_FAILURE);
    }

    /* loop reading each character in file */
    for (int c = fin.get(); !fin.eof(); c = fin.get()) {
        c = tolower(c);         /* convert to lower */
        if (c=='a' || c=='e' || c=='i' || c=='o' || c=='u')
            vowels++;
        else if (isalpha(c))    /* must be alpha to be consonant */
            cons++;
    }
    std::cout << "\nIn file " << ifname << " there are:\n  " << vowels 
                << " vowels, and\n  " << cons << " conansants\n";
}

(also worth reading Why is “using namespace std;” considered bad practice?)

Example Input File

$ cat dat/captnjack.txt
This is a tale
Of Captain Jack Sparrow
A Pirate So Brave
On the Seven Seas.

Example Use/Output

$ ./bin/vowelscons
enter filename: dat/captnjack.txt

In file dat/captnjack.txt there are:
  25 vowels, and
  34 conansants

Which if you count and classify each character gives the correct result.

Look things over and let me know if you have any questions.

Answer 6

I know that the following will be hard to digest. I want to show it anyway, because it is the "more-modern C++"-solution.

So, I will first think and develop an algorithm, and then use moderen C++ elements to implement it.

First to the algorithm. If we use the ASCII code to encode letters, then we will see the following:

We see that the ASCII code for uppercase and lowercase letters just differ in the lower 5 bits. So, if we mask the ASCII code with 0x1F, so char c{'a'}; unsigned int x{c & 0x1F}, we will get values between 1 and 26. So, we can calculte a 5 bit value for each letter. If we now mark all vowels with a 1, we can build a binary number, consisting of 32bits (an unsigned int) and set a bit at each position, where the vowel is true. We then get something like

Bit position
3322 2222 2222 1111 1111 1100 0000 0000  
1098 7654 3210 9876 5432 1098 7654 3210  
Position with vowels:
0000 0000 0010 0000 1000 0010 0010 0010

This numer can be converted to 0x208222. And if we now want to find out, if a letter (regardless whether upper- or lowercase) is a vowel, then we mask out the not necessary bits from the chararcter ( C & 1F ) and shift the binary number to the right as much position, as the resulting letter code has. If then the bit is set at the LSB position, then we have a vowel. This know how is decades old.

Aha. No so easy, but will work for ASCII coded letters.

Next, we create a Lambda, that will read a string that purely consists of alpha letters and counts the vowels. What is not a vowel, that is a consonant (because we have letters only).

Then we use modern C++ elements to calculate the requested values:

The result is some elegant C++ code with only a few lines.

Please see

#include <utility>
#include <algorithm>
#include <string>
#include <iostream>
#include <fstream>
#include <cctype>

int main() {

    // Lambda for counting vowels and consonants in a string consisting of letters only
    auto countVowelsAndConsonants = [](std::string& s) -> std::pair<size_t, size_t> {
        size_t numberOfVowels = std::count_if(s.begin(), s.end(), [](const char c) { return (0x208222 >> (c & 0x1f)) & 1; });
        return { numberOfVowels, s.size() - numberOfVowels }; };

    // Inform the user what to do: He should enter a valid filename
    std::cout << "\nCount vowels and consonants.\n\nEnter a valid filename with the source text:  ";

    // Read the filename
    if (std::string fileName{}; std::cin >> fileName) {

        // Now open the file and check, if it could be opened
        if (std::ifstream sourceFileStream(fileName); sourceFileStream) {

            // Read the complete source text file into a string. But only letters
            std::string completeSourceTextFile{};
            std::copy_if(std::istreambuf_iterator<char>(sourceFileStream), {}, std::back_inserter(completeSourceTextFile), std::isalpha);

            // Now count the corresponding vowels and consonants
            const auto [numberOfVowels, numberOfConsonants] = countVowelsAndConsonants(completeSourceTextFile);

            // Show result to user:
            std::cout << "\n\nNumber of vowels:     " << numberOfVowels << "\nNumber of consonants: " << numberOfConsonants << "\n\n";
        }
        else {
            std::cerr << "\n*** Error. Could not open source text file '" << fileName << "'\n\n";
        }
    }
    else {
        std::cerr << "\n*** Error. Could not get file name for source text file\n\n";
    }
    return 0;
}

Please note:

There are one million possible solutions. Everbody can do, what he wants.

Some people are still more in a C-Style mode and others do like more to program in C++