Thread: Error in my Code(Deletion(Linked Lists))

  1. #1
    Registered User
    Join Date
    Dec 2020
    Posts
    15

    Error in my Code(Deletion(Linked Lists))

    The Number is not deleting(Pls.Help)
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    
    typedef struct con {
        int num;
        struct con *next;
    } node;
    
    
    void take(node *list)
    {
        printf("\n\tEnter the Number(0 to END): ");
        scanf("%d",&(*list).num);
        if(list->num!=0)
                {
                    list->next = (node *)malloc(sizeof(node));
                    take(list->next);
                }
    }
    
    
    void printfrom(node *list)
    {
        printf("%d",list->num);
        if(list->next->num!=0)
        {
            printf("->");
            printfrom(list->next);
        }
    }
    
    
    void del(node *list,int a)
    {
    
    
        if((*list).num==a)
           {
                list = (*list).next;
           }
           else del((*list).next,a);
    }
    
    
    int main()
    {
        printf("\n\t\t\tDeletion(Linked Lists)\n");
        node *head;
        int a;
        head = (node *)malloc(sizeof(node));
        take(head);
        printfrom(head);
        printf("\n\tEnter the Number you want to remove: ");
        scanf("%d",&a);
        del(head,a);
        printfrom(head);
        return 0;
    }

  2. #2
    Registered User
    Join Date
    Sep 2020
    Posts
    167
    The pattern followed seems a little bit off.

    I would expect to only allocate a new node when you have a non-zero number.

    But for deleting, you have three cases.

    1. The item is the head of the list.

    2. The item is in the middle of the list.

    3. The item is not found.

    I can't see how you have all cases covered.

    Also, recursion (e.g. in printfrom()) is not the best way to handle linked lists.

  3. #3
    Registered User
    Join Date
    Sep 2020
    Posts
    167
    There is a bigger logical problem - what does an empty list look like?

  4. #4
    Registered User
    Join Date
    Sep 2020
    Posts
    167
    Oh, and if you call malloc() to allocate something, you should also be calling free() to release the memory back to the system.

  5. #5
    Registered User
    Join Date
    Dec 2020
    Posts
    15
    I want to know what is wrong in the above code.. bro

  6. #6
    Registered User
    Join Date
    Dec 2020
    Posts
    15
    I want to know what is wrong in the above code.. bro

  7. #7
    Registered User
    Join Date
    May 2009
    Posts
    3,970
    Casting malloc - Cprogramming.com

    The code is crap; throw it out and start over or use the advise and fix it!

    Asking over and over what is wrong is a waste of time!
    "...a computer is a stupid machine with the ability to do incredibly smart things, while computer programmers are smart people with the ability to do incredibly stupid things. They are,in short, a perfect match.." Bill Bryson

  8. #8
    Registered User
    Join Date
    Dec 2020
    Posts
    15
    Sorry to ask this again, but I am not getting it. I am unable to figure out what is wrong.. I am following a book: advanced topics in c by noel kalicharan. If I just let this go, then I will not learn what's the mistake. I know I am skipping some parts. I want to know what will happen now. Please Kindly grant my request. - XrossHAIR

  9. #9
    Registered User
    Join Date
    May 2009
    Posts
    3,970
    Find another book because you have no idea what you are doing!

    If I tell you to remove the Ace of Spades from a deck?
    What card would you remove?

    Edit: Please do not ever PM again!! If you do I will ask that you be banned!

    Tim S.
    Last edited by stahta01; 01-02-2021 at 02:47 AM.
    "...a computer is a stupid machine with the ability to do incredibly smart things, while computer programmers are smart people with the ability to do incredibly stupid things. They are,in short, a perfect match.." Bill Bryson

  10. #10
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,224
    hamster_nz went over a bunch of your mistakes, but you didn't seem inclined to go through them.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  11. #11
    Registered User
    Join Date
    Sep 2020
    Posts
    167
    Ok... let's see.

    Good points.

    It compiles without errors.

    Bad points - just about everything else.

    So let's start with the printfrom() function.

    Code:
    void printfrom(node *list)
    {
      printf("%d",list->num);
      if(list->next->num!=0)
      {
        printf("->");
        printfrom(list->next);
      }
    }
    What happens you try and print an empty list? It crashes.

    Why is it made recursive? With enough data in the list it too may crash the program! (please - nobody point out that tail recursion optimization can stop this)

    So let's fix that one . Here's the standard pattern for walking a linked list

    Code:
    void process_list(node *head) {
       node *current = head;
       while(current != NULL) {
          /* do something with the current node - in your case print it */
          current = current->next;
       }
    }
    It won't crash, and it will work with any length of list.

    Note: You can always see if you need to print a separator or not - if (current == head) then you are on the first item so no separator is needed.

    If you can change your printfrom() to something more like that, then show me your working and we will move on to how 'take()' is wrong.

  12. #12
    Registered User
    Join Date
    Dec 2020
    Posts
    15
    Here, I changed it.. @hamster_nz
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    
    typedef struct con {
        int num;
        struct con *next;
    } node;
    
    
    void take(node *list)
    {
        node *temp = list;
                    printf("\n\tEnter the Number(0 to END): ");
                    scanf("%d",&(*temp).num);
        while(temp->num!=0)
                {
                        temp->next = (node *)malloc(sizeof(node));
                        temp = temp->next;
                     printf("\n\tEnter the Number(0 to END): ");
                    scanf("%d",&(*temp).num);
                }
            temp->next = NULL;
    }
    
    
    void printfrom(node *list)
    {
        node *temp;
        temp = list;
        while(temp->next!=NULL)
            {
                printf("%d",temp->num);
                if(temp->next->next!=NULL)
                            printf("->");
    
    
                        temp = temp->next;
            }
    }
    
    
    void del(node *list,int a)
    {
        node *temp;
    if(list->num==a)
            {temp  = (*list).next;
                free(list);
                list = temp;
    }
    
    
       else
            del(list->next,a);
    }
    
    
    int main()
    {
        printf("\n\t\t\tDeletion(Linked Lists)\n");
        node *head;
        int a;
        head = (node *)malloc(sizeof(node));
        take(head);
        printfrom(head);
        printf("\n\tEnter the Number you want to remove: ");
        scanf("%d",&a);
        del(head,a);
        printf("\n\n\t\t");
        printfrom(head);
        return 0;
    }
    EDIT : I changed take() too. I hope it is the correct way.
    Last edited by XrossHAIR; 01-02-2021 at 03:26 AM.

  13. #13
    Registered User
    Join Date
    Sep 2020
    Posts
    167
    Looking good!

    Do you really want to add the zero to the list?

  14. #14
    Registered User
    Join Date
    Dec 2020
    Posts
    15
    Because I don't know how to end the take() without using it(ZERO).

    EDIT: should I free the last one(ZERO) and put previous pointer to null?

  15. #15
    Registered User
    Join Date
    Sep 2020
    Posts
    167
    I would do it another way... get the input into an int variable, and then only malloc and add the node when it is != 0.

    Code:
    void take(node **list_end)
    {
        while(1) {
            int a;
            node *new_node;
    
            // Get the user input
            printf("\n\tEnter the Number(0 to END): ");
            if(scanf("%d", &a) != 1)
                break;  // Input failed
            if(a == 0)
                break; // User entered a zero, or maybe bad input
    
            // Create a new node
            new_node = malloc(sizeof(node));
            if(!new_node)
                break;
            new_node->num = a;
            new_node->next = NULL;
    
            // Add it to the list
            *list_end = new_node;
            list_end = &(new_node->next);
        }
    }
    If you don't mind the list being in reverse order (older at the end) it gives much clearer and cleaner code.

    Code:
    void take(node **head)
    {
        while(1) {
            int a;
            node *new_node;
    
            // Get the user input
            printf("\n\tEnter the Number(0 to END): ");
            if(scanf("%d", &a) != 1)
                break;  // Input failed
            if(a == 0)
                break; // User entered a zero, or maybe bad input
    
            // Create a new node
            new_node = malloc(sizeof(node));
            if(!new_node)
                break;
            new_node->num = a;
    
            // Add it to the start of the list
            new_node->next = *head; // Next is current head.
            *head = new_node;   // Head is now the new node.
        }
    }

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Help in linked lists (probably a very stupid error)
    By Phantaminum in forum C Programming
    Replies: 11
    Last Post: 12-30-2014, 03:15 PM
  2. Replies: 3
    Last Post: 02-03-2014, 07:34 AM
  3. Replies: 1
    Last Post: 07-17-2012, 12:56 AM
  4. Linked Lists error: m_deck undeclared identifier
    By marQade in forum C++ Programming
    Replies: 17
    Last Post: 05-16-2008, 02:15 PM
  5. Sorting Linked Lists (code not concept)
    By Newbie Magic in forum C++ Programming
    Replies: 2
    Last Post: 05-11-2004, 08:57 AM

Tags for this Thread