2

So I'm trying to make a Ternary search trie. Right now, I'm only working on the insert function. I have understood the basic idea of a ternary search trie online. I know that one root node has 3 leaves, and if the character comes before the root, it goes to the left, after - right and if it matches the root, it goes to the middle leaf. So my main objective is to make a program that can suggest words for miss-spelled user entered words. But at the moment I am just working on making the ternary search trie. I use the trie to make a dictionary using which I check user entered words to suggest the next best alternative. But right now, just working on inputting some characters into the ternary trie and when I display it should display it in order. I am not 100% sure of my logic regarding the middle leaves. Now my program, when run, give me some garbage unlimited values somehow related to the last character entered. I don't know where I've gone wrong. Could someone please point out where I have made my mistake? Also, could you please tell me if any of the logic I've written is wrong? I mean, you don't have to give me the code or anything as I feel I am capable of doing it myself once I understand where I've gone wrong, but if someone could just help me find my mistakes, it would help me a lot!

The WHOLE code :

#include <stdio.h>
#include <stdlib.h>  //Because usage of malloc gives warnings without this header
typedef struct tstree
{
    struct tstree *lokid;
    struct tstree *hikid;
    struct tstree *eqkid;
    char letter;
}node;
node *head = NULL;
int count = 0;
int insert(char x, node **head)
{
    if (*head==NULL)            //If it's the first element
    {
        *head = malloc(sizeof(node));   
        (*head)->letter = x;
        count++;            
        (*head)->lokid = (*head)->hikid = (*head)->eqkid = NULL;            //Assign all 3 to null initially
    }
    else if ((*head)->letter == x)          //If equal, insert below
    insert(x , &((*head)->eqkid) );
    else if ((*head)->letter > x)   //If inserted char comes before current val, insert to left
    insert(x,&(*head)->lokid);
    else
    insert(x,&(*head)->hikid);              //Else to the right
    return 0;
}
void display(node *head)
{
    if(head)
    {
        display(head->lokid);               //print in infix order
        printf("%c ",head->letter);
        display(head->hikid);
    }
    //return 0;
}
int main()
{   
    int op;
    char num;
    printf("\nWelcome Fabiz. Hope it meets your expectations!\n");
    while(1)
    {
        printf("\n1. To insert an element\n2. Display the tree\n3. Exit\nOption :");
        scanf("%d",&op);
        switch(op)
        {
            case 1:
            {
                system("clear");
                printf("\nEnter the element : ");
                scanf(" %c",&num);
                insert(num,&head);
                break;
            }
            case 2:
            {
                system("clear");
                if(count == 0)
                printf("\nEmpty tree\n");
                else
                {
                    printf("Display in order..\n");
                    display(head);
                }
                break;
            }
            default: exit(0);
        }
    }
    return 0;
}

I am using Geany text editor and am on Linux Mint. I got a problem one where in the compiler just prints the last character I entered infinitely when I hit the display function. Any help will be very helpful! Thanks!

templatetypedef
  • 362,284
  • 104
  • 897
  • 1,065
Izy-
  • 1,041
  • 2
  • 16
  • 29

1 Answers1

3

Your display function is wrong. The loop condition never evaluates to false:

while(&(*head)->lokid!=NULL && &(*head)->hikid!=NULL)

This is not what you want. None of &(*head)->lokid or &(*head)->hikid will ever evaluate to NULL. If head is not NULL, then &(*head)->lokid is just the same address as *head plus the offset of lokid in a struct tstree. Note that your loop doesn't even have an update statement that could possibly make the condition false, and it doesn't have any break - it's doomed.

In fact, you don't even need a loop at all. To print it in order, this is all you need:

void display(node *head) {
    if (head) {
        display(head->lokid);
        printf("%c ", head->letter);
        display(head->hikid);
    }
}

Note that there's no purpose in passing a node ** (I changed that to a node *), and the return value should be void.

UPDATE

Your insert() function is correct, but you use the wrong variable in main. This assignment in the recursion base from insert() is causing unintended behaviour:

temp = *head = malloc(sizeof(node));

Notice that every time you hit the base case, you assign a new node to *head and temp, thus losing reference to whatever was stored in temp before. Then you call display(temp). Yes, you build the trie, but you are printing it starting in the last inserted node - not what you want.

Instead, you should call display with the global variable head, which is the correct root of your trie:

display(head);

The same happens when you call insert. You do not want to insert a new letter starting on the last added node, you want to add it starting it on the root. main() should have this instead:

insert(num, &head);

And while we're at it, note that temp is completely unnecessary. You don't need it. insert manipulates the global head by reference, temp is of no use here (and in fact it introduced a bug).

Changing these 2 lines in main is enough, tested it here, and it's working like a charm.

Filipe Gonçalves
  • 20,783
  • 6
  • 53
  • 70
  • Oh! Daymn. I had actually done that initially. This was just my experimentation. But yes, I changed it back to if (head) and made it single pointer. The reason why this is just a single pointer type function is because we don't really need to edit anything in the trie / add to the trie so there's no need for having the node's actual address, am I correct? Also, on implementing your suggestion, I am getting something, but it's only giving me the last character I had entered. – Izy- Feb 26 '14 at 17:28
  • Yep, since you never change the argument that was passed, there's no need to receive a pointer to pointer. – Filipe Gonçalves Feb 26 '14 at 17:30
  • Thank you very much for your reply. However, on implementing your suggestions, I'm still not getting my desired answer. It displays the last entered character. – Izy- Feb 26 '14 at 17:37
  • And you're not doing any pointer manipulation somewhere else? This is weird. Did you show all the code? – Filipe Gonçalves Feb 26 '14 at 17:54
  • Thanks for your time, Filipe! I made the two changes you suggested. But now, It's again the last element that I entered into the trie. Also, could you please explain to me why you're sending head when displaying but &head when inserting? As in, what do each of them contain exactly? As head is a double pointer and display requires only a single pointer while insert too requires a double pointer. When I used these, I wasn't 100% sure on what I was doing which is why I find it a bit complex to understand the difference between the two. Thanks for all your help, Filipe! – Izy- Feb 27 '14 at 04:50
  • 1
    @icyfox26 Basically, C has no built-in pass-by reference calls. If you want to call `f(a)` and see the modifications that `f()` made to `a`, you have to pass a pointer to `a`, that is, `&a`. This is what we're doing in `insert()`. Since you can modify the head in `insert()`, and you want these changes to be seen by the caller, you have to pass a pointer to the head. The head is a `node *`, so a pointer to the head is `node **`. Again, we do this to have pass by reference calls. If you passed `head` only, then changes in `head` by `insert` wouldn't be seen after returning. [continues] – Filipe Gonçalves Feb 27 '14 at 10:06
  • 1
    The display function is only reading the contents of `head`, so you don't need to pass it by reference. Let's see an example: `head` is a pointer, imagine it's the address 0x21. When you call `display(head)`, this address is copied; inside `display()`, `head` is 0x21, but if you changed `head` to, say, 0x30, only the local copy in `display` would be changed - the `head` in the caller would still be 0x21. This is ok for `display()`, but not for `insert` because we want to change `head`. We actually pass the address where the address `0x21` is stored (pointer to pointer). – Filipe Gonçalves Feb 27 '14 at 10:11
  • 1
    Now, if you change 0x21 to something else, **because you're writing into the address where the pointer from the caller is stored**, this will be seen even after the function returns. Hope it's clear now. – Filipe Gonçalves Feb 27 '14 at 10:11
  • Thank you very much for your detailed response. I understand it now. However, one last thing. My code still isn't displaying the desired result. It's again just printing out the last character I've entered into the trie. Heh, sorry for being such a bother ^_^ – Izy- Feb 27 '14 at 14:37
  • Done @Filipe Goncalves – Izy- Feb 27 '14 at 16:55
  • @icyfox26 I just copied your code and tested it. It's working here. Are you sure you recompiled the binary with the updated code? I added `a`, `b`, `c` and `d` and then displayed the trie. Output was as expected. Make sure you recompiled the code, or show me the exact test case where it fails. – Filipe Gonçalves Feb 27 '14 at 19:37
  • You're right. It works. I don't know why it didn't work before. Something weird. Anyway, thanks for all your help! Keep in touch. – Izy- Feb 28 '14 at 15:43