### Letter Count Function

So I need to decrypt a file, and my first step is to find the most common letter. I've written the following function to increment the letter count and it seems to be broken. Obviously this has yet to determine the largest element of the array, but that part should be easy once the first part is done.

 ``123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475`` ``````void count(int letters[26]) { //declare variables ifstream infile; char fileName[151]; // allow for a very long file location int index; char ch; cout << "Please enter the directory location of the file you wish to decrypt: " << endl; cin >> fileName; infile.open(fileName); // open the encrypted file while(!infile.eof()) { infile >> ch; ch = toupper(ch); switch(ch) { case 'A': letters[0]++; case 'B': letters[1]++; case 'C': letters[2]++; case 'D': letters[3]++; case 'E': letters[4]++; case 'F': letters[5]++; case 'G': letters[6]++; case 'H': letters[7]++; case 'I': letters[8]++; case 'J': letters[9]++; case 'K': letters[10]++; case 'L': letters[11]++; case 'M': letters[12]++; case 'N': letters[13]++; case 'O': letters[14]++; case 'P': letters[15]++; case 'Q': letters[16]++; case 'R': letters[17]++; case 'S': letters[18]++; case 'T': letters[19]++; case 'U': letters[20]++; case 'V': letters[21]++; case 'W': letters[22]++; case 'X': letters[23]++; case 'Y': letters[24]++; case 'Z': letters[25]++; default: break; } cout << letters[4]; } }``````
What is the problem? Or rather, what makes you think anything is wrong with it?
When run, the console displays endless 0's
Have you tried debugging it to see what the value of 'ch' is after each read?
Well, I recommend looking back at the documentation for switches. You'll plainly see what is wrong with your switch, though I'll leave the actual discovery of what's wrong to you.
After each case your should include break. For example

 ``123`` ``````case 'A': letters[0]++; break;``````

Also it will be better to select a required element of the array the following way without using the long switch statement

 ``12345`` ``````ch = toupper(ch); unsigned int index = ch - 'A'; if ( index < 26 ) ++letters[index];``````
Last edited on
The other way to determine a required element of the array is the following

 ``123456789`` ``````const char alphabet[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; .... ch = toupper(ch); const char *p = std::strchr( alphabet, ch ); if ( ch != 0 ) ++letter[p - alphabet];``````

So I've boiled it down to something similar to your suggestion, but my ending cout statment still results in an output of a long list of numbers, which seem to cycle up to four and five then repeat. The program exits in under ten seconds. Pausing the program after the first cout results in "00".

Here's the updated code:

 ``12345678910111213141516171819202122232425`` ``````void count(int letters[26]) { //declare variables ifstream infile; char fileName[151]; // allow for a very long file location int index; char ch; cout << "Please enter the directory location of the file you wish to decrypt: " << endl; cin >> fileName; infile.open(fileName); // open the encrypted file while(!infile.eof()) { infile.get(ch); ch = toupper(ch); index = static_cast(ch) - static_cast('A'); if (0 <= index && index < 26) ++letters[index]; cout << letters[21]; } }``````

Is it possible something is wrong with the file input? To me the function looks solid.

What does cout << letters[21]; mean? What is a stupid statement?!
Last edited on
It's to test that the function has incremented a count.
When change it to cout << letters[index];

And why are you inventing your own code as

 ``12`` ``````index = static_cast(ch) - static_cast('A');``````

Are you able to simply copy what I wrote?
Last edited on
Topic archived. No new replies allowed.