0

I know this is one of the most frequently asked questions here, since I' ve spent more than an hour reading posts by people that faced similar problems. But still, I can't tell what's going on in my situation.

What I know:

  • My code compiles locally
  • Program runs as expected (when compared with the solutions given by the cs50 staff)
  • Valgrind shows no memory related errors

enter image description here

  • I didn't change anything in speller.c and/or dictionary.h. I only wrote the functions in the dictionary.c file.
  • Since I read the posts of many people that faced similar issues with this pset, I realized that at some point check50 was broken, but the comments stating this are more than 1 year old by now, so I highly doubt it they haven't fixed that yet.

What I DON'T know:

  • Why check50 fails to compile my code even when I check with check50 locally (check50 cs50/problems/2024/x/speller -l) (expected exit code 0, not 2)
  • Since my code can't compile (according to check50), I don't even know if it passes all the tests, because every single one of them says "can't check until a frown turns upside down".

What check 50 cs50/problems/2024/x/speller returns

Here is my code:

// Implements a dictionary's functionality #include <ctype.h> #include <stdbool.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <strings.h> #include "dictionary.h" // Represents a node in a hash table typedef struct node { char word[LENGTH + 1]; struct node *next; } node; // TODO: Choose number of buckets in hash table (Additional comment: each bucket is a linked list) const unsigned int N = 4500; // Hash table node *table[N]; int words_amount = 0; bool is_loaded = false; // Returns true if word is in dictionary, else false bool check(const char *word) { // TODO int bucket_indx = 0; char *lowercase_word = malloc((sizeof(char)) * (strlen(word) + 1)); strcpy(lowercase_word, word); for (int i = 0; lowercase_word[i] != '\0'; i++) { lowercase_word[i] = tolower(lowercase_word[i]); } bucket_indx = hash(lowercase_word); node *cursor = table[bucket_indx]; while (cursor != NULL) { if (strcasecmp(cursor->word, lowercase_word) == 0) { free(lowercase_word); return true; } else { cursor = cursor->next; } } free(lowercase_word); return false; } // Hashes word to a number unsigned int hash(const char *word) { // TODO: Improve this hash function int hash_addition = 0; char *lowercase_word = malloc(strlen(word) + 1); if (lowercase_word == NULL) { return false; } strcpy(lowercase_word, word); for (int i = 0; lowercase_word[i] != '\0'; i++) { lowercase_word[i] = tolower(lowercase_word[i]); } int length = strlen(lowercase_word); for (int i = 0; i < length; i++) { hash_addition = hash_addition + (lowercase_word[length - i] * lowercase_word[i]); } free(lowercase_word); return hash_addition % N; } // Loads dictionary into memory, returning true if successful, else false bool load(const char *dictionary) { // TODO char buffer[LENGTH + 1]; node *n = NULL; // Open the dictionary file FILE *source = fopen(dictionary, "r"); if (source == NULL) { printf("Error: Could not open file.\n"); return false; } // Copy each word read from the file to a new node else { while (fscanf(source, "%s", buffer) != EOF) { n = malloc(sizeof(node)); { if (n == NULL) { fclose(source); return false; } } // Convert word in buffer to lowercase, character by character for (int i = 0; buffer[i] != '\0'; i++) { buffer[i] = tolower(buffer[i]); } strcpy(n->word, buffer); int index = hash(buffer); n->next = table[index]; table[index] = n; words_amount++; } is_loaded = true; fclose(source); return true; } } // Returns number of words in dictionary if loaded, else 0 if not yet loaded unsigned int size(void) { // TODO if (!is_loaded) { return 0; } else { return words_amount; } } // Unloads dictionary from memory, returning true if successful, else false bool unload(void) { // TODO int i = 0; node *cursor = NULL; node *tmp = NULL; for (i = 0; i < N; i++) { cursor = table[i]; while (cursor != NULL) { tmp = cursor; cursor = cursor->next; free(tmp); } } // At this point cursor should be NULL anyway, so checking if i= N is enough to find out if all // the nodes are freed if (i == N) { return true; } else { return false; } } 

Thanks in advance to anyone willing to help me!

4
  • 1
    please post here the exact errors from check50 not via a link.. from your description the issue does not sound like a compiler error it sounds like an error in what you are returning somewhere. Commented Mar 15, 2024 at 19:01
  • 1
    why are you converting the check word to lowercase if you are just going to use strcasecmp? if you are converting it then you shouldnt need strcasecmp.... smart use of strcpy but dont forget you have typing issues... look at the type of return of hash and look at the type of the variable you are storing it into. Also you are not assigning a NULL when you do your strcpy's to the last character so you are going to run into problems eventually. thats a bug. Commented Mar 15, 2024 at 19:06
  • 2
    Two things: there is an "incident with codespaces" message on the cs50 status page. It could be a timing problem with your check, or it could have nothing to do with anything. The other thing: I copied this dictionary.c into my codespace, (including fresh download of the distro), compiled it (success) and ran check50, no errors reported (locally and not). Commented Mar 15, 2024 at 21:17
  • @DinoCoderSaurus Thanks for your feedback, it really helps knowing that my code works as expected! Even if check50 disagrees... I still don't know why though. The CS50 status page shows that all services are up and running, but I get the exact same error. I will try to do what you did in case I had changed something in the other files by mistake. Commented Mar 16, 2024 at 12:23

2 Answers 2

1

Well, it turned out that I had made changes in the other 2 files unintentionally (dictionary.h or speller.c).

I incorporated most of the suggestions UpAndAdam posted, but ultimately the solution was simpler and DinoCoderSaurus really helped me troubleshoot the issue. I downloaded speller.zip again and pasted the code in my dictionary.c file and the issue is resolved (check50 now says my code compiles and passes all the tests).

enter image description here

I have to admit I feel quite stupid, as I have been changing things in my code for two days now and I hadn't tried the easiest solution, which was just checking if the other two files of the program were intact. It seems that I had changed something in there while reading the code without realizing.

Big thanks to @UpAndAdam and @DinoCoderSaurus that took the time to help me solve the issue!

2
  • We do not permit posting of full solutions -- thus undermining the integrity of the course. Commented Mar 21, 2024 at 23:40
  • 1
    Thank you for the call outs to Dino and myself, best thing to do to thank us is also to upvote our comment and answer. I have to concede of course that Dino's suggestion of pulling clean copy of the other files was a stellar idea to remove a variable! Commented Mar 22, 2024 at 14:00
1

I don't see what would cause you to fail to compile but I see a couple of things you should fix:

  1. type mismatches. You have hash returning an unsigned int, but you don't use unsigned int inside the function or to capture the return value. This is a bug.
  2. You don't NULL Terminate the strings you everytime you use strcpy; this is a bug. Also strongly advise you not to use strcpy, but use strncpy instead.
  3. You have an odd logical issue where you do a bunch of extra work for no benefit. You copy and lower the string in the hash function, which you just copied and lowercased in the check function, and then you still use strcasecmp out in check.. this is very very very silly. You already do the lowercasing in check, and load prior to calling hash so hash has no need to do it again at all. Additionally it means you don't have use strcasecmp and can just use strcmp because you've already converted the words. You're welcome.
  4. in your unload just return i==N no need for if else there
  5. you might want to consider what size should be after unload. yours is definately wrong. You may also consider utilizing size as a check that you are freeing everything and there isn't a bug in your logic somewhere.
  6. why do you need is_loaded? just initialize the count variable to 0, grow it and shrink it as needed, now the size method doesnt have to do any logic. ( which it shouldnt have to do)
  7. lastly why read into buffer only to have to copy it into a node's storage for the word. Why not just read directly into that storage to start with and avoid the copy?

good luck and hope this helps.

If this answers your question, please click on the check mark to accept it. Please consider upvoting the answer as well if it was useful. This helps with site maintenance.

1
  • Thanks for your feedback. I changed some things according to your suggestions, but the issue persists. I know that I convert to lowercase more than needed, but my mindset was that it's better to make sure than all words are lowercase everywhere, even if I have to do some extra work. I changed the hash function so that length is an unsigned int as well (I hadn't noticed, thanks for pointing this out) and replaced strcasecmp with strcmp. I also included an extra condition in the unload function to check that size returns 0 after all nodes are freed. But still, it won't compile. Commented Mar 16, 2024 at 12:19

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.