0

I thought it was a good idea to write a function to create a new node because it seemed to come up more than once. I named it new_node() and it looks like this:

struct node *new_node(void)
{
    node *new_node = malloc(sizeof(node));
    new_node->is_word = false;

    for (int i = 0; i < 27; i++)
    {
        new_node->branch[i] = NULL;
    }

    return new_node;
}

I removed checking for NULL from within the function because it generated an error as return 1; is an int and I currently have new_node() returning a struct node.

I can't figure out what I'm missing because the side effect is I'm checking for NULL both before and after I call new_node(). Here's the excerpt:

// loop to read char from file one at a time
for (int c = fgetc(fp_dictionary); c != EOF; c = fgetc(fp_dictionary))
{
    // for chars which are not a new line
    if (c != '\n')
    {
        // calls alpha function to convert decimal to alphabetic index
        alpha(&c);

        // checks if root node's branch ptr at alpha index is NULL
        // 'root' is dynamically declared on the heap and therefore, uses -> to dereference
        if (trav->branch[c] == NULL)
        {
            // creates new node if ptr at index c is NULL
            trav->branch[c] = new_node();

            // quits if ptr returned for new_node is NULL
            if (trav->branch[c] == NULL)
            {
                return 1;
            }

            // advance in trie to be ready for next char
            trav = trav->branch[c];
        }

        else if (trav->branch[c] != NULL)
        {
            // move to existing node and continue
            trav = trav->branch[c];
        }
    }

I have been made aware of calloc, but as it can return NULL, it doesn't seem to get me out of this. Am I interpreting the man page wrong? I'd be so happy if it were the case. I considered when trav was assigned the value of trav = trav->branch[c];, the loop took care of the NULL check, but it never exits the program if it were to come back true. Is this an issue or no? I feel as though I've blown something simple out of proportion. Thanks for the help.

1 Answer 1

1

You say: "I can't figure out what I'm missing because the side effect is I'm checking for NULL both before and after I call new_node(). "

You aren't missing anything. You have to first check if it's null, because, if it is, you want to create a new node. Then, you have to check that malloc didn't fail and you actually have a node before you add it to your trie. There's nothing wrong there.

One thing I'd add is that if the new node is NULL, you should probably call your unload() to unload any nodes you've already created, and then return false (rather than 1, only because load is declared as a bool function).

1
  • Thanks for confirming it was necessary to check both times--it was starting to feel as though I was obsessing on screen. Recommendation to call unload() and return false is great. I'm so used to returning 1, I've been very slow on the uptake re: considering whether it's what I really want returned or if there's a better option. Really appreciate it.
    – Lindsey
    Oct 8, 2017 at 4:47

You must log in to answer this question.

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