0

A couple questions here.

I'm trying to figure out Week 5 Pset5 Speller. Here is my code:

// Implements a dictionary's functionality

#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>

#include "dictionary.h"

// Represents a node in a hash table
typedef struct node
{
    char word[LENGTH + 1];
    struct node *next;
}
node;

// Number of buckets in hash table
const unsigned int N = 1000;

// Hash table
node *table[N];

// Dictionary size
int dictionary_size = 0;

// Returns true if word is in dictionary, else false
bool check(const char *word)
{
    // TODO #4!
    
    // make lowercase copy of word
    char copy[strlen(word) + 1];
    for (int i = 0; i < strlen(word); i++)
    {
        copy[i] = tolower(word[i]);
    }
    
    // get hash value
    int h = hash(copy);

    // use hash value to see if word is in bucket
    if (table[h] != NULL)
    {
        node *temp = table[h];
        
        while (temp != NULL)
        {
            if (strcmp(temp->word, copy) == 0)
            {
                return true;
            }
            
            temp = temp->next;
        }
    }
    
    return false;
}

// Hashes word to a number
unsigned int hash(const char *word)
{
    // TODO #2
    // source: https://www.reddit.com/r/cs50/comments/1x6vc8/pset6_trie_vs_hashtable/cf9189q/
    // I used this source because I had trouble understanding different variations - this one explained everything well.
    // I modified it slightly to fit my needs
    unsigned int hash = 0;
    for (int i = 0; i < strlen(word); i++)
    {
        hash = (hash << 2) ^ word[i];
    }
    return hash % N;
}

// Loads dictionary into memory, returning true if successful, else false
bool load(const char *dictionary)
{
    // TODO #1!
    // open dictionary file
    FILE *file = fopen(dictionary, "r");
    if (file == NULL)
    {
        return false;
    }
    
    // read strings from file one at a time
    char word[LENGTH + 1];
    while (fscanf(file, "%s", word) != EOF)
    {
        node *n = malloc(sizeof(node));
        if (n == NULL)
        {
            return false;
        }
        
        // place word into node
        strcpy(n->word, word);
        
        // use hash function to take string and return an index
        int h = hash(word);

        // make the current node point to the bucket we want
        n->next = table[h];
        
        // make the bucket start now with the current node
        table[h] = n;
        
        //count number of words loaded
        dictionary_size++;
    }

    return true;
}

// Returns number of words in dictionary if loaded, else 0 if not yet loaded
unsigned int size(void)
{
    // TODO #3!
    return dictionary_size;
}

// Unloads dictionary from memory, returning true if successful, else false
bool unload(void)
{
    // TODO #5!
    for (int i = 0; i < N; i++)
    {
        while (table[i] != NULL)
        {
            node *temp = table[i]->next;
            free(table[i]);
            table[i] = temp;
        }
    }
    return true;
}

Question Part 1:

When I run Check50, the only error that shows is the last one:

:) dictionary.c exists
:) speller compiles
:) handles most basic words properly
:) handles min length (1-char) words
:) handles max length (45-char) words
:) handles words with apostrophes properly
:) spell-checking is case-insensitive
:) handles substrings properly
:( program is free of memory errors
    valgrind tests failed; see log for more information.

When I check Help50 Valgrind, it says:

==658== Conditional jump or move depends on uninitialised value(s)

Looks like you're trying to use a variable that might not have a
value? Take a closer look at line 70 of dictionary.c.

Valgrind then asks me to look at line 70, but I can't figure out what it's upset with. (Line 70 is the "for" line in the hash function). I've also tried moving the "strlen(word)" portion to be its own variable in case that's what it was upset about, and it still didn't agree with what I had done:

unsigned int hash(const char *word)
{
    // TODO #2
    // source: https://www.reddit.com/r/cs50/comments/1x6vc8/pset6_trie_vs_hashtable/cf9189q/
    // I used this source because I had trouble understanding different variations - this one explained everything well.
    // I modified it slightly to fit my needs
    unsigned int hash = 0;
    int j = strlen(word);
    for (int i = 0; i < j; i++)
    {
        hash = (hash << 2) ^ word[i];
    }
    return hash % N;
}

Can someone explain what valgrind is asking me to do?

Question Part 2: this part has been fixed :) see comments if you are curious about what was fixed in this portion of my question

Although Check50 is mostly happy with my code, I have run it multiple times and found my results do not match the staff results. For example, when I run ./speller texts/lalaland.txt, I get 2476 misspelled words while the staff got 955 misspelled words. I have the same number of words in the dictionary, but something in my code is not working the way it should. Can anyone help me identify where my problem is?

Much appreciated!!

  • 6
    `copy[i] = tolower(word[i]);` You are not nul terminating `copy` so it is not a valid C string when passed to `hash`. This results in `int j = strlen(word);` reading pass the end of what was written into `word` as there is no NUL terminator to tell it to stop, – kaylum Jul 14 '21 at 21:53
  • @kaylum yay, that fixed the problem in my second question!! Thank you so much for finding that! Now to try and fix the memory errors... – kvelastegui Jul 14 '21 at 22:01
  • I'm surprised it didn't fix your first problem as well. Is valgrind still giving the exact same error on the same line? – kaylum Jul 14 '21 at 22:02
  • @kaylum Actually, no, Help50 Valgrind shows "Sorry, help50 does not yet know how to help with this!" but Check50 still shows the same sad face error and tells me to check valgrind – kvelastegui Jul 14 '21 at 22:07
  • So, have you got Valgrind on your machine? If not, why not? Get it on there, and run a debug build of the program (add `-g` to both the compilation of the `.o` files and the linking process to build a debug version) under Valgrind. Then track what it says. – Jonathan Leffler Jul 14 '21 at 22:49
  • 1
    kvelastegui, What is `LENGTH`? What is longest word read? – chux - Reinstate Monica Jul 14 '21 at 23:27
  • 2
    kvelastegui Why search the string `word` multiple times for its length with `for (int i = 0; i < strlen(word); i++)`. Consider the speedier `for (int i = 0; word[i]; i++)`. – chux - Reinstate Monica Jul 14 '21 at 23:34
  • @chux-ReinstateMonica, LENGTH is 45 (it was given as that in the project - I am not supposed to change it). – kvelastegui Jul 15 '21 at 01:25
  • @chux-ReinstateMonica, Thanks for the tip! I didn't know that you could do that, I'll try it out! :) – kvelastegui Jul 15 '21 at 01:25
  • @JonathanLeffler, I have been using Valgrind on the CS50 IDE. I'm just a beginner, so I'm not sure what you mean about the rest of your comment - is there any extra info you could give me so I can learn? Thanks – kvelastegui Jul 15 '21 at 01:34
  • 1
    regarding: `const unsigned int N = 1000; // Hash table node *table[N];` This does not compile! suggest changing: `const unsigned int N = 1000;` to `#define N 1000` Note: it will compile with a C++ compiler – user3629249 Jul 15 '21 at 02:24
  • OT: the function: `strlen()` returns a `size_t` not a `int` – user3629249 Jul 15 '21 at 02:28
  • regarding: `char copy[strlen(word) + 1]; for ( size_t i = 0; i < strlen(word); i++) { copy[i] = tolower(word[i]);` the function: `tolower()` returns a `int`, not a `char` – user3629249 Jul 15 '21 at 02:31
  • regarding: `int h = hash(copy);` at this point in the code, the function: `hash()` is not defined. Suggest placing a prototype before the first function in the code – user3629249 Jul 15 '21 at 02:33
  • regarding: `unsigned int hash(const char *word)` and `unsigned int hash = 0;` it is a very poor programming practice to make local variable names the same as the function name – user3629249 Jul 15 '21 at 02:42
  • strongly suggest enabling the warnings when compiling, then fixing those warnings – user3629249 Jul 15 '21 at 02:44
  • @user3629249, I should have mentioned that this is only the code in the file that I am to be manipulating for this project. There is another file that implements these functions. So the compiling problem and the hash function not being defined ahead of the check function is not a problem when the program is actually run. I have fixed the local variable names/function name problem that you mentioned - very good point! I'm still learning :) I'm looking into the return values you mentioned now. Thanks for all your help!! – kvelastegui Jul 15 '21 at 03:21

0 Answers0