1

I wrote a function for counting frequency of specific word in a text.This program every time return zero.How can I improve it?

while (fgets(sentence, sizeof sentence, cfPtr))
{
for(j=0;j<total4;j++)
        {
            frequency[j] = comparision(sentence,&w);
            all_frequency+=frequency[j];
}}
.
.
.
int comparision(const char sentence[ ],char *w)
{  
    int length=0,count=0,l=0,i;
    length= strlen(sentence);
    l= strlen(w);
    while(sentence[i]!= '\n')
    if(strncmp(sentence,w,l))
        count++;
    i++;
    return count;
    }
user2500540
  • 43
  • 1
  • 3
  • 2
    I'm surprised your program even returns. You use an uninitialized `i` which is never incremented in your `while(sentence[i]!= '\n')` because your `i++;` falls outside of the loop scope due to a lack of curly braces. – i Code 4 Food Jun 22 '13 at 04:08
  • It's also not clear what w is, how the frequency array is initialized, nor what total4 is about. This is not a good demonstration of the problem. – catfood Jun 22 '13 at 04:11
  • w is a word that taken from user. total4 is number of paragraphs. – user2500540 Jun 22 '13 at 04:16
  • Improve it by writing better code? ;) This is such a homework question. Should this even be on SO? – Adrian Jun 22 '13 at 04:21
  • yes it is a peace of my homework. can you help me? – user2500540 Jun 22 '13 at 04:28
  • No he won't,in fact no body would help doing your homework but you. – 0decimal0 Jun 22 '13 at 06:27

1 Answers1

2

I have proofread your code and have commented on coding style and variable names. There is still a flaw I left with the conditional, which is due to not iterating through the sentence.

Here is your code marked up:

while(fgets(sentence, sizeof sentence, cfPtr)) {
    for(j=0;j<total4;j++){
        frequency[j] = comparision(sentence,&w);
        all_frequency+=frequency[j];
    }

}

// int comparision(const char sentence[ ],char *w)  w is a poor variable name in this case.

int comparison(const char sentence[ ], char *word)  //word is a better name.
{

    //int length=0,count=0,l=0,i;   

    //Each variable should get its own line.
    //Also, i should be initialized and l is redundant.
    //Here are properly initialized variables:

    int length = 0;
    int count = 0;
    int i = 0;

    //length= strlen(sentence);   This is redundant, as you know that the line ends at '\n'

    length = strlen(word);  //l is replaced with length.

    //while(sentence[i]!= '\n') 

    //The incrementor and the if statement should be stored inside of a block 
    //(Formal name for curley braces).

    while(sentence[i] != '\n'){
        if(strncmp(sentence, word, length) == 0)  //strncmp returns 0 if equal, so you       
            count++;                              //should compare to 0 for equality
        i++;
    }
    return count;
}
jcccj1
  • 41
  • 3
  • this code has a basic problem: in every while_loop the counter counts every characters. how can I solve this problem? – user2500540 Jun 22 '13 at 05:52
  • This program has a basic problem. In every while_loop the counter counts every characters.How can I solve this problem? I know that use "strncmp" function in this program is not correct. – user2500540 Jun 22 '13 at 05:56
  • While using strncmp is a poor choice for this problem, it is quite possible. (I would have used strtok to grab each word delimited by a space). By using pointer arithmetic, we may offset sentence by a value of i, allowing you to effectively walk the list searching for the word. Thus, we may change the first argument of strncmp to (sentence + i) to solve the problem in this fashion. However, this solution is not context-sensitive, so if we are looking for "fire" and we find "firefighter", it will count that as fire being in the text. – jcccj1 Jun 22 '13 at 06:02