HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Simple program to insert values in a linked list

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simpleinsertprogramvalueslistlinked

Problem

I tried to write a simple program using linked list where I just insert the value stored in every node and then through a function print the values.

This is the code:

#include
#include

typedef struct Node {
    int value;
    struct Node* link;
} Node;

void print_node(Node * );

int main() {

    Node* head = NULL;
    head=malloc(sizeof(Node));
    if (head==NULL) {
       return 0;
    }
    printf("Insert the number of nodes :\n");
    int N;
    scanf("%d",&N);
    int i;
    Node * Node_c=head;
    for (i=0;ivalue=temp;
            Node_c->link=malloc(sizeof(Node));
            Node_c=Node_c->link;
        } else if (i=N-1) {
            int temp;
            printf("Insert value stored into the node %d:\n",i+1);
            scanf("%d",&temp);
            Node_c->value=temp;
            Node_c->link=NULL;
        }
    }
    printf("\n");
    print_node(head);
    return 0 ;
}

void print_node(Node * head) {
    Node * Node_current = head;
    int i=1;
    while (Node_current != NULL) {
        printf("Value stored into node %d:  %d\n",i,Node_current->value);
        Node_current = Node_current->link;
        i++;
    }
}


The code runs just fine but I want to know if this part where I insert the values stored in each node can be done in an shorter way

Node * Node_c=head;
for(i=0;ivalue=temp;
        Node_c->link=malloc(sizeof(Node));
        Node_c=Node_c->link;
    } else if (i=N-1) {
        int temp;
        printf("Insert value stored into the node %d:\n",i+1);
        scanf("%d",&temp);
        Node_c->value=temp;
        Node_c->link=NULL;
    }
}

Solution

I want to know if this part where I insert the values stored in each node can be done in an shorter way

Definitely it can be done in a shorter way!

-
the problem is that in your code you are taking the pain to do the same thing twice while inserting. in the for loop :

for (i=0;ivalue=temp;
        // to here

        Node_c->link=malloc(sizeof(Node));
        Node_c=Node_c->link;

    } 
    else if (i==N-1) //Note: it must be == operator not =
    { 

        //the same thing again from here 
        int temp;
        printf("Insert value stored into the node %d:\n",i+1);
        scanf("%d",&temp);
        Node_c->value=temp;
        //to here

        Node_c->link=NULL;

}


-
The below part appears twice in your insertion logic :

int temp;
    printf("Insert value stored into the node %d:\n",i+1);
    scanf("%d",&temp);
    Node_c->value=temp;


-
Now can we avoid writing twice the same thing?

... Definetely you can write that logic only once and change your insertion logic to :

int i;
Node * Node_c=head;
for(i=0;ilink=malloc(sizeof(Node));
        Node_c=Node_c->link;
    }

    //you don't require temp as you can directly access Node_c->value
    printf("Insert value stored into the node %d:\n",i+1);
    scanf("%d",&Node_c->value);

    Node_c->link=NULL;

}


the logic is quite simple to understand :

  • first we create a memory for a node and save it's address to the link of previous node(we don't if its a head node as it's already allocated with memory in the before part of the code and it has no previous nodes to be linked with)



  • then we take in the value and store in the value member of structure Node_c



  • then we point the link member of the structure Node_c to NULL... this goes on for N number of times

Code Snippets

for (i=0;i<N;i++) 
{
    if (i<N-1)
    {

        //here
        int temp;
        printf("Insert value stored into the node %d:\n",i+1);
        scanf("%d",&temp);
        Node_c->value=temp;
        // to here

        Node_c->link=malloc(sizeof(Node));
        Node_c=Node_c->link;

    } 
    else if (i==N-1) //Note: it must be == operator not =
    { 

        //the same thing again from here 
        int temp;
        printf("Insert value stored into the node %d:\n",i+1);
        scanf("%d",&temp);
        Node_c->value=temp;
        //to here

        Node_c->link=NULL;

}
int temp;
    printf("Insert value stored into the node %d:\n",i+1);
    scanf("%d",&temp);
    Node_c->value=temp;
int i;
Node * Node_c=head;
for(i=0;i<N;i++)
{
    if(i!=0) //not required once again for head
    {
        Node_c->link=malloc(sizeof(Node));
        Node_c=Node_c->link;
    }

    //you don't require temp as you can directly access Node_c->value
    printf("Insert value stored into the node %d:\n",i+1);
    scanf("%d",&Node_c->value);

    Node_c->link=NULL;

}

Context

StackExchange Code Review Q#132967, answer score: 3

Revisions (0)

No revisions yet.