3
\$\begingroup\$

I have written the code below for inserting an element at the position specified by the user.

I have seen the code in multiple books, and they have always used two pointers in the function that inserts the node at specified position. However, in my code I have used only single pointer.

My code works fine, but I want to know if there any pitfalls to this design.

#include<stdio.h>
#include<stdlib.h>
struct node{
        unsigned int data1;
        unsigned int  data2;
        struct node *ptr;
}obj;

void enterData()                                                        // Here the EnterDAta fnnction uses the obj object to enter the data and note that this
{                                                                       // obj is used agauin and again in the every node of the list to enter the data
        printf("\n Enter the data1 ");
        scanf("%u",&obj.data1);
        printf("\n Enter the data2 ");
        scanf("%u",&obj.data2);
}
void append(struct node **start)                                        // This                                                                     is used to append the dara un the list or also used to add the first element in                                                                     the list
{
        enterData();
        struct node *next_node=*start;
        if(next_node==NULL)
        {
                printf("\nAdding first element in the list ......\n");
                next_node=malloc(sizeof(struct node));
                if(next_node==NULL)
                {
                        printf("\n Out of Memory");
                }
                else{
                        next_node->data1=obj.data1;
                        next_node->data2=obj.data2;
                        next_node->ptr=NULL;
                        *start=next_node;                                      //This line of code here is modifying the header pointer see the magic of the poiter :)
                }
                printf("\n The first element added successfully");
        }
        else
        {
                printf("\n Appending the data ......\n");
                struct node *temp=next_node;
                next_node=malloc(sizeof(struct node));
                if(next_node==NULL)
                        printf("\n Out of Memory");
                else
                {
                        next_node->data1=obj.data1;
                        next_node->data2=obj.data2;
                        next_node->ptr=NULL;
                        while(temp->ptr!=NULL)
                                temp=temp->ptr;
                }
                temp->ptr=next_node;
                temp=NULL;
                printf("\n Data appended Successfully!!! ");

        }
next_node=NULL;
}

void display(struct node *start)                                               //Used to display the list
{
        if(start==NULL)
                printf("\n The list is empty!!. Please create a list firsr Using                                                                     option 1 !!! ");
        else
        {
                printf("\n The linked list is as shown below ");

                        while(start!=NULL)
                        {
                                 printf("[%d | %d] ",start->data1,start->data2);
                                 start=start->ptr;
                                if(start!=NULL)
                                        printf("->");
                        }
        }
        start=NULL;
}
void insertmid(struct node *start)  // This is the function inserting the nodes at the specified position enetered by the user
{
        struct node *q=start,*new_node;
        int k=1,po,total=0,i=1,j=1;
        printf("\n Enter the position where u want to enter the new node ");
        scanf("%d",&po);
        while(k<po-1)
        {
                q=q->ptr;
                ++k;
        }
        new_node=malloc(sizeof(struct node));
        enterData();
        printf("\n Adding the node at the specified position !!!!!  ");
        new_node->data1=obj.data1;
        new_node->data2=obj.data2;
        new_node->ptr=q->ptr;
        q->ptr=new_node;


}

int main(void)
{
        struct node *head=NULL;
        append(&head);
        append(&head);
         append(&head);
         append(&head);
         append(&head);
        display(head);
        insertmid(head);
        display(head);
        insertmid(head);
        display(head);
        insertmid(head);
        display(head);
        insertmid(head);
        display(head);
        insertmid(head);
        display(head);
        insertmid(head);
        display(head);
        insertmid(head);
        insertmid(head);
        display(head);
        insertmid(head);
        display(head);
        insertmid(head);
        display(head);

        return 0;
}
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Avoid global data, such as the obj variable. There is really no need for it in this program. You could make enterData take as parameter a pointer to a node, and fill the node fields directly. Actually your entire program could become a lot simpler, by eliminating all the places where you manually copy the fields from obj to fill new nodes.

When appending a new node, much of the code is duplicated for the two cases of having a head or not. You could reduce the duplication, as the code to create the node is the same in both cases, the only difference is in how you stick the node in the list. Move the code that creates the node out of the conditional.

When printing the limited list, it's ugly to check start != NULL twice: once in the loop condition and once to decide to print an arrow or not. Also note that the loop condition is always true for the first time. A better solution is to print the start element before the loop, and is there is a next element, then print an arrow and the element. This removes the unnecessary check inside the loop.

As for inserting a node, I don't know what is this thing you've read about two pointers, but there is no need for that, one pointer is enough, naturally. However, you don't handle the case of inserting at the front.

\$\endgroup\$
2
  • \$\begingroup\$ Well i saw some of the programs and everywhere they have used two pointer for inserting the node at specified position one pointer is pointing to the node previous to the node to be inserted while other pointer is pointing to the node next to the node which will be next to the new node. As I have used only a single pointer I wanted to ask is this also fine or I missed something ? \$\endgroup\$ Commented Jun 29, 2015 at 6:34
  • 1
    \$\begingroup\$ You did fine for this part. \$\endgroup\$
    – janos
    Commented Jun 29, 2015 at 6:35
4
\$\begingroup\$
void append(struct node **start)                                        // This                                                                     is used to append the dara un the list or also used to add the first element in                                                                     the list

I don't know what you see, but I see a 304-character line with loads of spaces.

This is not good.

Here's something a little more approachable:

// This is used to append the dara un the list or
// also used to add the first element in the list
void append(struct node **start)

Further, what is "dara un"? A typo for "data in"?

\$\endgroup\$
1
  • \$\begingroup\$ Yes it was a typo for data in \$\endgroup\$ Commented Jun 26, 2015 at 15:16

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.