0

I've rewritten my code a bunch of times and it clicks in my head...buuuuut it doesn't actually work.

I'm not doing the actual cs50 problem because I prefer to start smaller then adapt to cs50.

I'm pretty sure my load and check work fine but could anyone recommend an improvement to unload or something i'm missing. thank you.

#include <ctype.h>
#include <stdio.h>
#include <sys/resource.h>
#include <sys/time.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>

#define CAP 5

bool insert(char* word);
bool check (char* word);
bool unload(void);

typedef struct node
{
    bool word;
    struct node *next[CAP];
}
node;

node root;

void del (node *ptr);

int main (void)
{
    char* wordd = "bba";
    char* worddd = "aab";

    insert(wordd);
    insert(worddd);
    unload();
}

node *current = &root;

bool unload (void)
{
    del (current);
    return true;
}

void del (node *trav)
{
    for (int i = 0; i < CAP; i++) //iterate over all children
    {
        node *tmp; //temporary blah blah
        if (trav->next[i] == NULL)
        {   

        }
        else
        {
            for (int y = 0; y < CAP; y++)
            {
                if (trav->next[i]->next[y] == NULL)
                {

                }
                else if (trav->next[i]->next[y] != NULL)
                {
                    trav=trav->next[i];
                    return del(trav);
                }
            }
            tmp = trav->next[i];
            free(tmp);
        }

    }
}
0

First of all the ifs with the empty body are really inelegant, and no serious programmer does this. If you want to code something in the else case, then change the condition to it's opposite. So this

if (trav->next[i] == NULL)
{
    // no code
}
else
{
    // code
}

can turn into this:

if (trav->next[i] != NULL)
{
    // code
}

and it will be much cleaner.

A second error you have is the fact that in del() that is declared as void, you have the line

return del(trav);

which tries to return a value.

Also I don't understand why there are two for loops in del(). You should loop on each letter of trav like you correctly do, and if trav->next[i] is not NULL, call del() on it. After you do this for all trav->next[], call free() on trav.

After you do this and your code works, you should consider if the recursive approach is the best solution for this case.

Happy Coding!

2
  • void del (node *trav) { for (int i = 0; i < CAP; i++) { if (trav->next[i] != NULL) { trav=trav->next[i]; del(trav); } } free(trav); } I did this but it gets one free then crashes because of a double free. I've tried to debug it many times.
    – NovNov
    Jun 15 '17 at 13:31
  • Delete the trav=trav->next[i];. You don't want to advance your pointer down the trie. It will automatically advance if you do del(trav->next[i]). At the end you want to free trav. If you advance trav to trav->next[i] you lose your hold of trav and you can't free it. And you also try to free trav->next[i] twice.
    – ChrisG
    Jun 16 '17 at 7:50
0

I think it's your return statement. I had the same issue with my unload function - the function unloaded one node in my trie, but then the return statement just exited everything in my loop. I was using a recursive function, and as soon as I changed return function() to just function() it worked.

1
  • my return was one of the problems but not the sole one (i was returning when its a void function) . I solved it by following ChrisG's suggestions.
    – NovNov
    Jun 19 '17 at 20:30

You must log in to answer this question.

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