I'm new to programming and we have a project in class asking us to have the user input a sentence then we need to find the character typed most in that sentence. I think I'm on the right track, but I'm getting a segmentation core dump while running the program.

The function I'm using looks like this:

void common(char * string, int x){
cout << "Enter a sentence no longer than " << (SIZE - 1) << " characters long." <<endl;

//Use ASCII Values to determine most common character
int a[255] = {0};
int i, mostCommon;

mostCommon = a[0];
i = 0;
for(i=0; &string != 0; i++)
    if (a[i] > mostCommon)
            mostCommon = a[i];
            x = i;
cout << "The most common letter in your sentence is " << (char)x << "." <<endl;

Any help or insight would be much appreciated.



The main problem is that you have no idea what is going on because you have not put print statements std::cout<<"I am here"<<std::endl; in your code.

Next you have a character string that you call string. You actually pass the pointer to the first character. In a typical c-style character string the last character is zero.

Now how do you access those characters, that you have to do by addressing it by one of several methods: Let us do a quick summary:

You can use array addressing, e.g.

char* Str="this is some string";
std::cout<<"The  fourth letter is : "<<Str[3]<<std::endl;

Note that arrays are offset from zero, so Str[3] refers to the forth character.

you might decide to access them by doing a pointer offset. See that Str [in the example above] points to the first character in the string. To see that you can do this:

char* Str="this is some string";
std::cout<<"The  first letter is : "<<*Str<<std::endl;
std::cout<<"The memory location of that letter is : "<<Str<<std::endl;

Not that the * operator is used [it is not multiply here... another confusing issue sorry], to indicate that the contents of the memory location are required.

Therefore you can do this:

char* Str="this is some string";
std::cout<<"The  forth letter is : "<<*(Str+3)<<std::endl;
std::cout<<"The memory location of that letter is : "<<Str+3<<std::endl;

So you have, effectively moved 4 characters further on, which is effectively the forth character.

Now, in your loop you take the address of a pointer and compare it with zero, that will always be true, hence your loop is infinite: You would have seen this if you had written std::cout<<"Index of loop == "<<i<<std::endl; in your loop.

However, to correct, you need something like this for(i=0;string[i]!=0;i++) Next, you simply have the wrong algorithm. Think about how you would do this with pen/paper. You would make a list of all the letters (e.g. a-z) and each time you see an 'a' you will add one to the 'a' count, and each time you see a 'b' add one to the 'b' count etc. Then after you have finished, you can look through your list to find the one that has the most.

That tells us you most likely need two loops. So I am going to leave you to come up with that.

Lastly off you haven't actually given us the whole code and I think that would have been important in this case. You may well have written : using namespace std; Unfortunately, unknown to you, the namespace includes string. Hence the possibility for some horrific errors. So please, remove that line, and either use the form I like e.g. the std::cout or use a specific using, e.g. using std::cout; for all the parts that you are using.

This article has been dead for over six months. Start a new discussion instead.