Thread: word occurence counter 'forgetting' about already saved word

The snippets below are from my program that gets words, then prints them with the number of occurences.
It works almost fine - it "forgots" that that entry has been saved before and does NOT increment counter associated with that. So I get eg. we - 1 we - 1 we - 1 instead of we - 3.

Code:

typedef struct { 
    char *word; 
    int occ; 
} 
words; 
words *data=NULL;

Code:

words *findword(char *word)
{
    words *ptr = data;
    
    if ((strcmp(word, ptr->word)) != 0)
            return ptr;
    return NULL;          
    
}

Code:

int main(int argc, char **argv) 
{ 
    char *word; 
    words *temp; 
    int c,i,num;
    FILE *infile; 
    words *ptr = NULL; 

    if(argc!=2) 
    { }         
    if((infile=fopen(argv[1],"r"))==NULL) 
    { } 
    num=0;
    data = malloc(sizeof(words));
        data->word = "";
        data->occ = 0; 
    while(1) 
    { 
        c=fgetc(infile); 
        if(c==EOF) break; 
        if(!isalpha(c)) continue; 
        else ungetc(c,infile); 
        word=getword(infile); 
            
        if(findword(word)) 
        { 
            
            if(!(temp=realloc(data,sizeof(words)*(num+1)))) 
            { } 
            else 
                data=temp;

            ptr = findword(word);
            data[num].word=strdup(word); 
            data[num].occ=1; 
            num++;
            
        } 
        else 
            ptr->occ++;                 
 
        free(word); 
    } 
   
    /* sort procedure here, irrelevant for the purpose of topic */
    for(i=0;i<num;i++) 
    { 
        printf("%s - %d\n",data[i].word,data[i].occ); 
        free(data[i].word); 
    } 
    free(data); 
    if(fclose(infile)) 
    {/* error handling */  } 
    return 0; 
}

What's wrong with that code?
Thanks in advance!

line23 getword is a function you made i guess.Shouldn't you post it to? strdup too

strdup is from <string.h>
I've only shown parts relevant to the problem. getword works fine, no need to make a mess in the post and extend it to hundreds of lines of code.
getword returns char *

OK. So following Your advice I've made:

Code:

words *findword(char *word)
{
    words *ptr = data;
    
    for (ptr; /*what to put as stop cond. */; ptr++) {
        if ((strcmp(word, ptr->word)) != 0)
            return ptr;
    }
    return NULL;          
    
}

What should be the stop cond. ?

Originally Posted by WaltP

Try a WHILE loop instead. Exit when either
a) word found or
b) end of word list

is TRUE

I don't know how to represent the end of constantly updated array of structures.

You'll have to keep a count of the length of the array in main and pass it in to findword.

My search function must have prototype struct abc *find(char *word)
so I can't pass that parameter.

Code:

words *search(char *word)  
{ 
    words *ptr = data;
    
    int i; 
    for(i=0; i< num; i++) 
    { 
        if(!strcmp(data[i].word,word)) return ptr; 
    } 
    return NULL; 
} 
... 
int main(int argc, char **argv) 
{ 
...
if((ptr = search(word))) {
    ptr->occ++;  /* if the word is found, then this ptr doesn't move; always increments the first encountered word in file. PROBLEM is here, but I don't know how to make it "moving" */

You are always returning the first element of data from search() because you set "ptr" to data[0] but never move it.
You should return &data[i] and you won't need "ptr" in search().

Bye, Andreas

Code:

data = calloc(100, sizeof(words)); /* initialized some memory
                                  because I was getting SIGSEGV */

I forgot:
Why do you allocate memory for 100 elements if you later reallocate it unconditionally anyway?
The likely problem for your seg fault is that you don't allocate memory for the initial word (at line 36). Allocating memory for 100 elements of "words" is the wrong solution.

Bye, Andreas

I did calloc (with a random value - I've chosen 100) so that I can initialize data->word = ""; data->occ = 0; . This was suggested to avoid SIGSEGV caused by strcmp in search function with a NULL word.

The problem with your code is that it is very inefficient.
You first allocate memory for an array of 100 structs. Then as soon as you store the first word in it, you shrink it to an array of one struct and later resize it only step by step.

The usual way (at least how I've learned it) is to start with a reasonable size and only resize it by a constant factor (for example double it) if you reach the limit:

Code:

int max = 10;  // for the current limit
int counter = 0; // for the used size
data = calloc(max, sizeof(*data));  // start with 10 elements;
...
if (counter == max)
{
    temp = realloc(data, sizeof(*data) * max * 2);  // try to get a bigger array
    if (temp == NULL) 
    { 
         // error handling
    }
    else
    {
         data = temp;
         max *= 2;  // new limit;
    }
}
// processing the new word

If you want to avoid using a global counter variable you could use a sentinel value (e.g. data->word = "") at the end of your array.

Bye, Andreas