1

I am trying to insert/delete the first element in a linked list but in case of insertion it is not adding it and in case of deletion it starts a infinite chain, I am unable to identify the issue. I am currently learning DSA so please ignore any unnecessary comments added in code

The code I created, (in C Language)

#include <stdio.h>
#include <stdlib.h>

struct Linked_List                      //structure has to be defined first
{
    int number;
    struct Linked_List *next;
};

typedef struct Linked_List node;

void create(node *p);
int count(node *p);
void print(node *p);
node *insert(node *head);
node *find(node *p, int key);
node *delete(node *head);

int main() {
    node *head;
    head = (node *)malloc(sizeof(node));
    create(head);
    printf("\n");
    print(head);
    printf("\n\nThe total number of elements in the linked list are :- %d",count(head));
    printf("\n");
    insert(head);
    printf("\n");
    print(head);
    printf("\n");
    delete(head);
    printf("\n");
    print(head);
}

void create(node *p) {
    printf("\nInput number (Input -999 to end) :- ");
    scanf("%d", &p->number);

    if (p->number == -999) {
        p->next = NULL;
    } else {
        p->next = (node *)malloc(sizeof(node));
        create(p->next);
    }
}

int count(node *p) {
    if (p->next == NULL) {            //using the if we are not counting the last element (-999)
        return 0;                   //return 1    will add the last element to total count
    } else {
        return 1 + count(p->next);
    }

    //return 1 + count(p->next);            //FAIL (as at last returns null pointer pointing to nowhere) returns total elements including -999,
}

void print(node *p) {
    
    // printf("%d --> ", p->number);        //this fails as in the end it will pass a null pointer which doesn't points to anywhere
    // print(p->next);
    
    if (p->next != NULL) {                        //this works as it first checks if pointer is null or not 
        printf("%d --> ", p->number);            //if not it prints the number and recursion occurs
        if (p->next->next == NULL) {              //this if statement is used to print the last value which holds null pointer as outer if will not process it,
            printf("%d", p->next->number);       //it checks for next value if null pointer then prints its value
        }
        print(p->next);
    }    
}

node *insert(node *head) {
    node *new;                                  //structure of list
    node *preceding;                            // preceding -> new -> key
    int x, key;

    printf("\nEnter the new value to be added :- ");
    scanf("%d", &x);
    printf("\nValue of key item (-999 if last) {-> new -> key}:- ");
    scanf("%d", &key);

    if (head->number == key) {                            //case for first position
        new = (node *)malloc(sizeof(node));
        new->number = x;
        new->next = head;
        head = new;
    } else {
        preceding = find(head, key);
        if (preceding == NULL) {
            printf("\nKey not found!!\n");
        } else {
            new = (node *)malloc(sizeof(node));
            new->number = x;
            new->next = preceding->next;
            preceding->next = new;
        }
    }
    return head;
}

node *delete(node *head) {
    int key;        //item to be deleted
    node *preceding;
    node *p;        //temporary node
    
    printf("\nEnter the item to be deleted :- ");
    scanf("%d",&key);

    if (head->number == key) {
        p = head->next;
        free(head);
        head = p;
    } else {
        preceding = find(head, key);
        if (preceding == NULL) {
            printf("\nKey not found!!\n");
        } else {
            p = preceding->next->next;
            free(preceding->next);
            preceding->next = p;
        }
    }
    return head;
}

node *find(node *p, int key) {
    if (p->next->number == key) {           //we know it is not the first so checking 2nd from head
        return p;
    } else {                                 //checking 3rd from head
        if (p->next->next == NULL) {
            return NULL;
        } else {
            find(p->next, key);              //recursive call
        }
    }
}

I tried to get the pointer to save the new location but for some reason it doesn't work as in case of the insert function as follows

new->next = head;
head = new;
6
  • The list has a dummy initial node. Did you come up with this implementation on your own or do you get this from an online DSA course? Commented Sep 2, 2023 at 14:08
  • @X_Abhishek_X Show the data entered or deleted from the list. Commented Sep 2, 2023 at 14:29
  • @chqrlie yes from a book E balgurusamy ANSI C Commented Sep 2, 2023 at 14:59
  • delete(head) does not modify head in main. Indeed, it cannot. You probably just need head = delete(head); (or change the api to pass &head) Commented Sep 2, 2023 at 15:01
  • @X_Abhishek_X: this book exposes very bad examples in the linked list chapter. I am afraid you should disregard it and use another source. Commented Sep 2, 2023 at 16:18

2 Answers 2

1

Your approach with the dummy last node that contains the value -999 can only confuse users of the list.

For example why the user itself shall allocate a dummy node when there is the function create?! Who does actually create the list the user or the function create?!

This prompt in the function create

printf("\nInput number (Input -999 to end) :- ");

is supposed that the value -999 will not be stored in the list.

In this case for example the behavior of the function print is unpredictable.

Let's consder the situation when the list contains only one dymmy node with the value -999.

In this case nothng will be outputted due to this if statement

if (p->next != NULL) {                        //this works as it first 

Now let's assume that the lst contains two nodes with for example values 1 and -999. In this case the both values will be outputted due to the nested if statement

if (p->next->next == NULL) {`

The function find has a bug. There is absent a return statement.in this part

    } else {
        find(p->next, key);              //recursive call
    }

Also the function find can invoke undefined behavior when the list contains only one node with the value -999 due to this statement

if (p->next->number == key) {           //we know it is not the first so checking 2nd from head

Why do you decde that we know that it is not the first node of the list?! The functon is called within the function insert like

preceding = find(head, key);

And how to insert a node if the list contans only one dummy node? Why do you decide that the user knows in any time that the list contains only the single dummy node? Do he has to call the function count every time when he is going to insert a new node?

And at last the function insert has to be called like

head = insert( head );

The same way the function delete also has to be called

head = delete( head );

Also you need to write a function that will free all the allocated memory when the list is not rquored any more.

You should rewrite all your code anew removing the dummy node from the list implementaton.

Sign up to request clarification or add additional context in comments.

3 Comments

I agree 100%. The dummy initial node is a bad idea. The code is from an indian book: as a matter of fact, the book does not exactly suggest the use of a dummy node, but they do allocate the initial node before calling create that initializes it, allocates the next node and recurses for its initialization... very bad example and broken teaching.
@chqrlie Usually indian books are of a low quality. For example in many books the functon main is declared like void main().
Yes, it is quite common, and in this case, the book is a collection of horrors, such as function prototypes inside the function bodies, lack of tests of scanf() or malloc() return values, advocating obsolete main() prototypes without a return type and invalid ones with a void return type... all in a 9th reprint dated 2009!
1

As far as I understood, you are returning head but you are not setting it in your main method. Do you mean to do something like:

head = insert(head);
printf("\n");
print(head);
printf("\n");
head = delete(head);

Also maybe you need a return in your find method:

return find(p->next, key);              //recursive call

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.