c string help

[jigglymig1]

I have to write a program that accepts a pointer to a c-string as its argument. The function should return the charater that appears most frequently in the string along with the number of times it appears. for example, if i input "hello dolly" the output would be: "l sppears 4 times"

I am just working on the basics and it prints out the last number, but counts the stuff right. should i use string compare or something to compare the strings correctly. heres the code

#include <iostream>
#include <cctype>
#include <cstring>
using namespace std;
int main()
{
	const int SIZE=40;
	const int LETTERS=26;
	char strings[SIZE];
	int numberOfTimes[LETTERS];
	int length;
	int max=0;
	cout << "enter the string" << endl;
	cin.getline(strings,SIZE);
	strlwr(strings);
	length = strlen(strings);

	char alphabet[] = {'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'};
	for(int i=0; i<LETTERS; i++)
	{
		numberOfTimes[i]=0;
		for(int j=0; j<length; j++)
		{
			if(alphabet[i]==strings[j])
			{
				numberOfTimes[i]+=1;
			}
		}
		if(numberOfTimes[i] > numberOfTimes[i-1])
		{
			max=i;
		}
	}


	cout << alphabet[max] << " appears " << numberOfTimes[max] << endl;
	

	return 0;
}

[vmanes]

Your counting of letter occurrence looks to be correct, but could be more efficient. Back to that in a bit.
This:

if(numberOfTimes[i] > numberOfTimes[i-1])
{
	max=i;
}

should be in a separate loop, after all the letter counting is done. Once you have the counts of letters, then scan this array looking for the max.
And, don't compare one element to its predecessor, compare to the stored value max . Initially, set max equal to the first element of numberOfTimes[].

Now, to the counting loop. For each letter of the alphabet, you are comparing to every character of the input string. That 26 * strlen(input) iterations. What if you turned that inside out, run the outer loop for the length of the string, and the inner loop checks against each letter of the alphabet. And make that inner loop stop if it's found a match, so it won't have to go a full 26 cycles every time.

Better yet, look for a way to use each letter of the input as a direct index into the numberOfTimes array and update the counts - no inner loop needed.

[r.stiltskin]

This

if(numberOfTimes[i] > numberOfTimes[i-1])
		{
			max=i;
		}
	}

does not identify the letter with the highest count. It identifies the LAST letter that has a higher count than the letter preceding it.

Instead, initialize max = numberOfTimes[0] and then search through the rest of numberOfTimes for higher values.

By the way, it's very wasteful to search for all 26 letters in every string. That means that for every 10-letter word you are doing 260 comparisons (and you are probably doing many of those comparisons after you have already finished counting the letters that are actually in the word). There is a much more efficient way to find the index of a particular letter in your numberOfTimes array (and the alphabet array is unnecessary). Hint: look at an ASCII chart. Or, what is the value of 'a' as an int.

[vmanes]

r. stiltskin, you reading my mind?

[r.stiltskin]

Hmmm ... guess I was just a bit slower. Great minds think alike? :)