I want to load a dictionary using TRIE
. The first three parts of code worked as expected. But I tried to shorten the third part of the code, then it didn't work, not adding nodes
to the TRIE
.
Here is the declaration part:
#define ALPHABET_SIZE 27
// define a node for a TRIE.
struct node
{
_Bool end_of_word;
struct node *next[ALPHABET_SIZE];
};
// create a TRIE
struct node *root = malloc(sizeof(struct node));
// create a mover.
struct node *mover = root;
// read dictionary file
FILE *dictptr = fopen(dictionary, "r");
The main thing starts from here:
//load dictionary word by word;
char c;
while ((c = fgetc(dictptr)) != EOF)
{
if (c == '\n')
{
mover->end_of_word = 1;
mover = root;
}
Here's where I want to optimize:
else
{
if (c == '\'')
{
mover->next[ALPHABET_SIZE - 1] = malloc(sizeof(struct node));
mover = &mover->next[ALPHABET_SIZE - 1];
}
else
{
mover->next[c - 97] = malloc(sizeof(struct node));
mover = &mover->next[c - 97];
}
// check if memory allocation is successful.
if (mover == NULL)
{
unload();
fprintf(stderr, "unable to allocate memory to new node.\n");
return false;
}
}
And here's what I optimized:
else
{
if (c == '\'')
{
mover = &mover->next[ALPHABET_SIZE - 1];
}
else
{
mover = &mover->next[c - 97];
}
mover = malloc(sizeof(struct node));
By doing what you did you "detached" the target of assignment from the recipient lvalue in mover->next[...]
, thus destroying the original functionality of the code. In your version mover->next[...]
remains unchanged.
If one literally wanted to eliminate the code duplication here, one could've done something as follows
struct node **pmover;
if (c == '\'')
pmover = &mover->next[ALPHABET_SIZE - 1];
else
pmover = &mover->next[c - 97];
mover = *pmover = malloc(sizeof(struct node));
This would be a literal implementation of your intent, which can also be rewritten as
struct node **pmover = &mover->next[c == '\'' ? ALPHABET_SIZE - 1 : c - 97];
mover = *pmover = malloc(sizeof(struct node));
Although a better idea, in my opinion, would be
struct node *new_node = malloc(sizeof(struct node));
if (c == '\'')
mover->next[ALPHABET_SIZE - 1] = new_node;
else
mover->next[c - 97] = new_node;
mover = new_node;
(or an equivalent ?:
-based version).