0

I have looked over a bunch of posts on speller.c load but I cannot figure out what I am doing wrong here. My load function seems to be working well as a linked list. My check function keeps returning all words as misspelled. It has to be something about how I am generating word_copy (and it not matching the dictionary word), but I can't figure it out.

Any help would be appreciated.

bool check(const char* word)
{
int check_hash;         
//get hash value of word to be checked
if(isupper(word[0]))
{
    check_hash = word[0] - 65;
}
else if(islower(word[0]))
{   
    check_hash = word[0] - 97;
}      
else
{
    return false;
}

//create a an array to hold the lower case copy of word
int word_length =  strlen(word)+1;
char word_copy[word_length];

//create lower case version to compare to dictionary
for(int s = 0; s < word_length; s++)
{
    if(isalpha(word[s]))
    {
        word_copy[s] = tolower(word[s]);
    }      
    else
    {
        word_copy[s] = word[s];
    }
}

//create cursor node to compare words
node* cursor_node;

if(hashtable[check_hash] == NULL)
{
    return false;
}  
else
{
    cursor_node = hashtable[check_hash];   

    //cycle through words and compare check if word is in the dictionary 
    while(cursor_node != NULL)
    {   
        if(strcmp(cursor_node->dict_word, word_copy) == 0)
        {
            return true;
            break;
        }
        else
        {
            cursor_node = cursor_node->next;
        }
    }
    return false;
}
}

Thanks. I will try to shorten check a bit.

Here is load. The one issue I had with load is that my while loop seems to be going around one extra time and producing a "key" value of -97, causing an error. I put a short-term fix in.

bool load(const char* dictionary)
{  
//create hashtable and initialize it
for(int n = 0; n < 27; n++)
{
    hashtable[n] = malloc(sizeof(node)); 
    hashtable[n] = NULL;
}  

// open dictionary file 
FILE* dict = fopen(dictionary, "r");

//check file 
if (dict == NULL)
{
    printf("Could not open %s.\n", dictionary);
    return false;
}
else
{
    //scan each word in and place it in the hashed linked list  
    while(!feof(dict))  
    {
        //Allocate memory for a new word       
        node* new_node = malloc(sizeof(node));
        new_node -> next = NULL;             

        //copy each word into new_node
        fscanf(dict, "%s", new_node->dict_word);

        //determine which hash the word goes to
        int key = new_node->dict_word[0] - 97;

        if(key < 0)
        {
            break;
        }

        //temporary prints out the key value as each word is loaded
        //printf("%i", key);
        //printf("\n");

        //update the word counter
        word_count++;
        //temporary - checks number of words loaded in the dictionary
        //printf("%d", word_count);
        //printf("\n");

        //if hashtable is empty for that letter, make word first node of hashtable
        if(hashtable[key] == NULL)
        {
            hashtable[key] = new_node;
        }
        else
        {
            //if hashtable is not empty, add node to chain
            new_node -> next = hashtable[key];
            hashtable[key] = new_node;   
        }                            
        free(new_node);
     }  
}
fclose(dict);
return true;

}

1
  • Your problem could easily be in the load code. Without seeing it, there's no way to tell. BTW, you could simplify your check in a couple spots. The hash could be simplified to if(isalpha(word[0])) check_hash = tolower(word[0]) - 97; else... Also, the break statement following the return is unnecessary. The return immediately exits check() and no further processing will be done in check.
    – Cliff B
    Commented Nov 2, 2015 at 18:39

1 Answer 1

0

You're freeing new_node. That's the node where you have stored the word, the one you have inserted into the hashtable, etc. Freeing it in the load function is not a good idea.

Also, and i'm not sure how much this will influence this, but it will, at least cause a memory leak:

for(int n = 0; n < 27; n++)
{
    hashtable[n] = malloc(sizeof(node)); 
    hashtable[n] = NULL;
} 

You malloc a block of memory for hashtable[n] and then you set hashtable[n] to NULL, effectively losing the pointer to the malloced memory. The malloced memory will remain but that's not what hashtable[n] is pointing at, and now you can't possibly free the malloced block

1
  • Thank you, this seems to have worked.
    – bkline
    Commented Nov 5, 2015 at 12:36

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .