Letter Count Function

[futureshocked]

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.

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];
	}
}

[Zhuge]

What is the problem? Or rather, what makes you think anything is wrong with it?

[futureshocked]

When run, the console displays endless 0's

[Zhuge]

Have you tried debugging it to see what the value of 'ch' is after each read?

[Ispil]

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.

[vlad from moscow]

After each case your should include break. For example

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

ch = toupper(ch);

unsigned int index = ch - 'A';

if ( index < 26 ) ++letters[index];

[vlad from moscow]

The other way to determine a required element of the array is the following

const char alphabet[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";

....

ch = toupper(ch);

const char *p = std::strchr( alphabet, ch );

if ( ch != 0 ) ++letter[p - alphabet];

[futureshocked]

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:

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<int>(ch)
				 - static_cast<int>('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.

[vlad from moscow]

What does cout << letters[21]; mean? What is a stupid statement?!

[futureshocked]

It's to test that the function has incremented a count.

[vlad from moscow]

When change it to cout << letters[index];

And why are you inventing your own code as

index = static_cast<int>(ch)
	 - static_cast<int>('A');

Are you able to simply copy what I wrote?