Inserting nodes in a sorted manner in a Linked List

Question

I'm trying to create a sorted list in ascending order. I must read a file which contains a year in every line ( so I want to order from the earliest date to the most recent).

What I'm trying to accomplish with my code is:

1. List item
2. Retrieve data (year) from a line of the .csv file;
3. Iterate over the list until the place for the node containing data is found;
4. Repeat until the file is over;
5. Print;

Whenever I try to run it, the Virtual box starts lagging and doesn't do anything. (even when I remove the print function).

I've been trying to solve this for 3 days now, so I'm quite desperate.

Here's my code:

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

struct node {
int data;
int key;
struct node *next;
} node;


struct node *head_countries = NULL;
struct node *current = NULL;

//display the list
void printList() {
   struct node *ptr = head_countries;
   printf("\n[ ");


    //start from the beginning
    while(ptr != NULL) {
    printf("(%d,%d)",ptr->key,ptr->data);
    ptr = ptr->next;
}

printf(" ]");
}

//insert link at the first location
struct node* insertFirst(int key, int data) {
    //create a link
    struct node *link = (struct node*) malloc(sizeof(struct node));

    link->key = key;
    link->data = data;

    //point it to old first node
    link->next = head_countries;

    //point first to new first node
    head_countries = link;

    return link;
    }

void insertAfter(struct node* PrevNode, int key, int data)
{
    struct node *NewNode = (struct node*) malloc(sizeof(struct node));

    if ( PrevNode == NULL )
    {
        printf("Erro 1");
        return;
    }

    NewNode->key = key;
    NewNode->data = data;

    NewNode->next = PrevNode->next;
    PrevNode->next = NewNode;

}


void CreateCountriesList()
{
    char linha[150];
    char cabecalho[100];
    int key = 0, data = 0;
    int test[15] = {0};

    test[0] = 10;
    test[1] = 25;
    test[2] = 7;
    test[3] = 5;
    test[4] = 30;
    test[5] = 40;
    test[6] = 3;
    test[7] = 4;
    test[8] = 98;
    test[10] = 4;
    test[11] = 14;
    test[12] = 23;
    test[13] = 16;
    test[14] = 35;
    test[15] = 6;

    //dados_temp New_Entry;

    struct node* curr_head = NULL;
    struct node* curr = NULL;

    FILE *inputf;
    inputf = fopen("tempcountries_all.csv", "r");


    if (inputf == NULL)
    {
        printf("Nao da pa abrir o ficheiro");
        exit(EXIT_FAILURE);
    }

    fgets(cabecalho, 100, inputf);

    for (key = 0; key <  20 ; key++)
    {
        data = test[key]

    if ( head_countries == NULL || key == 2 )
    {
        insertFirst( key, data );
    }

    else
    {
        curr = head_countries;
        //insertAfter(curr, key, data);
        //printf("%d\n", curr->data);
        //curr = curr->next;

        while ( curr->next != NULL )
        {
            //printf("%d", key);
            if ( curr->data < data && curr->next->data > data )
            {
                insertAfter(curr, key, data);
            }

            if ( data == curr->data )
            {
                //insertAfter(curr, key, data);
            }

            curr = curr->next;
        }
    }


}

printList();

fclose(inputf);
}

int main() {
CreateCountriesList();
return EXIT_SUCCESS;
}

Is it because the list is too big? If so, how do you recommend I proceed for a list this big?

Thank you in advance!

EDIT: removed warnings from code, and unused functions.

EDIT: added test.

Answer 1

You have several problems, but the most significant one seems to revolve around how you insert data into the list:

    while ( curr->next != NULL )
    {
        //printf("%d", key);
        if ( curr->data < data && curr->next->data > data )
        {
            insertAfter(curr, key, data);
        }

        if ( data == curr->data )
        {
            //insertAfter(curr, key, data);
        }

        curr = curr->next;
    }

You do not break from the loop after performing an insertion, so observe what happens:

• You find an appropriate predecessor, P, for the data you want to insert. curr will at that time point to P.
• You insert the a new node, N, after P.
• You continue iterating through the list. The next node you consider is the successor to P, which is now N.
• N also meets the conditions for predecessor to your data (data == N.data), so you insert another new node. And another. And another ...

This will continue indefinitely, and the computer is indeed likely to start slowing down before too long, as its physical and virtual memory become loaded down with all the nodes the program allocates.

Answer 2

So i guess your biggest problem is to make that list...

This is how i would make those functions.

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


typedef struct node {
int data;
int key;
struct node *next;
} node;


node *new_node(int key, int data)
{
    node * n =malloc(sizeof(node));
    if(!n)
    {
        fprintf(stderr,"malloc failed");
        exit(EXIT_FAILURE);
    }
    n->data = data;
    n->key = key;
    n->next = NULL;
}

node *insert_middle(node *next, int key, int data)
{
    node *new = new_node(key,data);
    new->next = next;
    return new;
}


node * insert_node(node *n, int key, int data)
{
    if(!n)
    {
        return new_node(key, data);
    }
    else if(n->data < data)
    {
        n->next = insert_node(n->next, key, data);
    }
    else
    {
        return insert_middle(n, key, data);
    }

}

int main()
{
    node *head=NULL;
    node *tmp = NULL;
    int i,j;
    for(i=0;i<10;i++)
    {
        scanf("%d",&j);
        tmp = insert_node(head, 0, j);
         //keep track of the head
        if(tmp!=head)
            head = tmp;
    }
    printf("====================");
    while(head)
    {
        printf("%d\n",head->data);
        head=head->next;
    }
    return 0;
}

Some bugs in your code i have found:

for (key = 0; key <  20 ; key++)
{
    //you never change the value of data
    data = test[0];

if ( head_countries == NULL || key == 2 )
{
     //you should place curr = insertFirst(key, data);
     //the way you have written the code nothing happens
    //inserFirst function just return the vale and you don't store it anywhere so you can not use it
    insertFirst( key, data );
}

else
{
    curr = head_countries;
    //after this line of code curr=NULL in first iteration 

     //curr->next will cause SIGSEGV since curr=NULL, so NULL->next is not possible
    while ( curr->next != NULL )
    {
        //printf("%d", key);


        if ( curr->data < data && curr->next->data > data )
        {
            insertAfter(curr, key, data);
        }
        if ( data == curr->data )
        {
            //insertAfter(curr, key, data);
        }

        curr = curr->next;
    }

Too many bugs... I think you should start again. If you dont understand my example just comment and i will give my best to explain