CODEWITHSUNDEEP
cpointerslinked-list
gizgok
gizgok

Reputation: 7649

What is wrong with my code for singly linked list?

I'm working on a singly linked list in C. This is what I've written so far.

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

struct Node{
    int value;
    struct Node *next; 
};


struct Node* init()
{
   struct Node* head=NULL;
   head=malloc(sizeof(struct Node));
    head->value=-1;
    return head;

}



int length(struct Node* head)
{
    struct Node* current=head;
    int length=0;
    while(current!=NULL)
    {
        length++;
        current=current->next;

    }
    return length;

}


void print(struct Node* head)
{
   int i=0;
   int len=length(head);
   for(i=0;i<len;i++)
   {
    printf("%d%d",i,head[i].value);
    printf("\n");


   }


}




 struct Node* insert(int data,struct Node* head)
 {
    struct Node* current=NULL;
    if(length(head) > 0)
    {
        int val=head->value;        
        if (val==-1)
        {
            head->value=data;
            head->next=NULL;

        }
        else
        {
           current=malloc(sizeof(struct Node));
           current->value=data;
           current->next=head;
           head=current;


      }

  }
   else
  {
    printf("List is empty");

  }

  return head;


}

int main() 
{

/* printf("Hello"); */
struct Node *head=init();

head=insert(20,head);
head=insert(30,head);
head=insert(40,head);

print(head);
printf("%d",length(head)); 

return 0;

}

The output values I get are: Index Value 0 40 1 0 2 0

and for length is 3. I'm not able to grasp what I'm doing wrong here in pointer manipulation.

Upvotes: 0

Views: 154

Answers (4)

Sashank Alladi
Sashank Alladi

Reputation: 13

The notation head[i].value is only valid for arrays but not for linked lists. Arrays and linked lists are completely different, allocation of memory towards arrays is premeditated where as for linked lists it's dynamic. That is the reason why we use pointers for linked lists.

In init() you didn't assign null which causes the loop to run infinite times when you call length() for first time.

I am posting the modified code of print function:

void print(struct Node* head)
{
    int i=0;
    int len=0;
    struct Node* current=head;
    for(i=0;i<len;i++)
    {
        printf("%d %d",i,current->value);
        print("\n");
        current=current->next;
    }
}

Upvotes: 0

Alexey Frunze
Alexey Frunze

Reputation: 62068

Here:

    for (i = 0; i < len; i++)
    {
        printf("%d%d", i, head[i].value);
        printf("\n");
    }

You need to advance from one node to another with head = head->next in the same manner as you do it in length(). head[i] won't do it.

It's unclear why your init() and insert() are so unnecessarily complicated and I don't even want to try to guess why. I want to suggest a better insert() and no init():

struct Node* insert(int data, struct Node* head)
{
    struct Node* current;

    current = malloc(sizeof(struct Node));
    current->value = data;
    current->next = head;

    return current;
}

And then you do this:

int main(void)
{
    struct Node *head = NULL;

    head = insert(20, head);
    head = insert(30, head);
    head = insert(40, head);

    print(head);
    printf("%d", length(head));

    return 0;
}

Upvotes: 1

Felice Pollano
Felice Pollano

Reputation: 33252

The Init function should set Next to NULL

struct Node* init()
{
   struct Node* head=NULL;
   head=malloc(sizeof(struct Node));
    head->value=-1;
    head->next=NULL;
    return head;

}

otherwise the first call to length return an undefined result ( or GPF ).

Upvotes: 1

Ofir
Ofir

Reputation: 8362

One obvious problem is not setting next to NULL on init - that would fail when checking length on the empty list

But your real problem is the print function

You can't use:

head[i].value

That notation is only valid for arrays, you need to use next to find each member

Upvotes: 4

Related Questions