Sorting Linked List Alphabetically while new nodes are being scanned in (C)

Question

I have been working on this project for a week straight and it's due at midnight tonight. I'm a freshman computer science student and new to programming so I cannot see what's wrong with this code. The code is supposed to read in event titles, event times, and event dates into a linked list and sort is alphabetically using the event titles.

This is the input from the file:

Birthday
12 30
10 01 2018
Wedding
06 30
06 15 2018
Seminar
05 00
02 15 2019
Birthday
04 00
06 15 2018
Anniversary
08 30
12 09 2019

For some reason, it never connects the wedding event to the linked list and also when printing the entire linked list there is a blank node as the head node. I have been working on this forever and even when tracing the code I cannot figure out what's going wrong.

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

//struct for event time
typedef struct{

  int hour;
  int minute;

} event_time_t;

//struct for event date
typedef struct{

  int month;
  int day;
  int year;

} event_date_t;

//struct for all event info
struct event{

  char event_title[20];
  event_time_t event_time;
  event_date_t event_date;
  struct event *next;

};

typedef struct event event_t;

void add_events (event_t **head_ptr);
void print_event(event_t *head_ptr);
void print_slected_event (event_t *head_ptr, int month, int day, int year);

int main () {

  event_t *head_ptr = malloc(sizeof(event_t));

  add_events(&head_ptr);

  print_event(head_ptr);

  print_slected_event(head_ptr,6,15,2018);

  return 0;
}

void add_events (event_t **head_ptr){

  event_t *temp;
  event_t *temp_head = *head_ptr;
  event_t *new_node;

  scanf(" %s",temp_head->event_title);
  scanf("%d",&temp_head->event_time.hour);
  scanf("%d",&temp_head->event_time.minute);
  scanf("%d",&temp_head->event_date.month);
  scanf("%d",&temp_head->event_date.day);
  scanf("%d",&temp_head->event_date.year);
  temp_head->next = NULL;

  while(!feof(stdin)){

    new_node = malloc(sizeof(event_t));

    scanf(" %s",new_node->event_title);
    scanf("%d",&new_node->event_time.hour);
    scanf("%d",&new_node->event_time.minute);
    scanf("%d",&new_node->event_date.month);
    scanf("%d",&new_node->event_date.day);
    scanf("%d",&new_node->event_date.year);



    if(temp_head->next == NULL){

      temp_head->next = new_node;

    }

    else if(strcmp(temp_head->event_title,new_node->event_title)>0){

      new_node->next = temp_head;
      temp_head = new_node;
      *head_ptr = temp_head;

    }

    else{

      temp = temp_head;

      while(temp->next!=NULL){

        if(strcmp(temp->event_title,new_node->event_title)==0){

          break;
        }
        if(strcmp(temp->event_title,new_node->event_title)<0){

          break;

        }

        temp = temp->next;
      }


        new_node->next = temp->next;
        temp->next = new_node;


    }


  }

}

void print_event(event_t *head_ptr){

  event_t *temp;
  temp = malloc(sizeof(event_t));
  temp = head_ptr;

  printf("Scedule of Events:\n");
  while(temp->next!=NULL){

    printf("\t%-13s at: %02d:%02d ",temp->event_title,temp->event_time.hour,temp->event_time.minute);
    printf("on: %02d/%02d/%d\n",temp->event_date.month,temp->event_date.day,temp->event_date.year);

    temp = temp->next;
  }
}

void print_slected_event (event_t *head_ptr, int month, int day, int year){

  event_t *temp;
  temp = malloc(sizeof(event_t));
  temp = head_ptr;

  printf("Date: %02d/%02d/%d\n",month,day,year);
  printf("Events:\n");
  while(temp->next!=NULL){

    if(temp->event_date.month == month){
      if(temp->event_date.day == day){
        if(temp->event_date.year == year){

          printf("\t%-13s at: %02d:%02d\n",temp->event_title,temp->event_time.hour,temp->event_time.minute);

        }
      }
    }

    temp = temp->next;
  }
}

Fyi, [`while(!feof(stdin))` is wrong](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). Better you know now rather than later. None of your actual IO reads are validated whatsoever, which is a *huge* mistake. — WhozCraig, Apr 08 '20 at 11:24
My suggestion is a big redesign. Instead of using one function which reads data, insert into the list, and then sort the list, split it into multiple smaller functions with limited responsibility. Follow the [KISS](https://en.wikipedia.org/wiki/KISS_principle) and [single-responsibility](https://en.wikipedia.org/wiki/Single-responsibility_principle) principles. So one function which reads *one* record from the file, one function which insert data into the list, and one function to sort the list. — Some programmer dude, Apr 08 '20 at 11:26
And fyi, neither of your `print` functions should be making *any* allocations whatsoever. The first three lines of both functions immediately leak memory (the second and third line in concert, to be accurate). — WhozCraig, Apr 08 '20 at 11:39
When you print, you shouldn't test `temp->next` for null, but `temp`. This makes you miss your wedding event. (Printing and inserting are different in that printing only needs to look at the current node and inserting must look at two consecutive nodes, so that it can update their links.) — M Oehm, Apr 08 '20 at 11:54
@WhozCraig ok I did some reading to find out why `!feof(stdin)` is always wrong and it made a lot of sense surprisingly, so I changed it to be `while(scanf(" %s",new_node->event_title)==1)` which got rid of that random empty head node. I also got rid of the print functions allocating memory to a temporary node. @M Oehm that makes way more sense and when I changed it, it instantly started printing the wedding event. Thank you guys so much, ever since classes got canceled linked lists have been hard, but I have a much better understanding now because of reading your suggestions. — dan, Apr 08 '20 at 12:22

Vlad from Moscow · Answer 1 · 2020-04-08T12:15:40.327

A function that inserts a node in the list shall be separated from the code that reads data from a file or fills structures with data.

I can suggest the following approach.

//struct for event time
typedef struct{

  int hour;
  int minute;

} event_time_t;

//struct for event date
typedef struct{

  int month;
  int day;
  int year;

} event_date_t;

//struct for all event info
typedef struct{
  char event_title[20];
  event_time_t event_time;
  event_date_t event_date;
} event_t;

//struct for node of the list
typedef struct node
{
    event_t event;
    struct node *next;
} node_t;

//struct for the list itself
typedef struct
{
    node_t *head;
} list_t;

In this case the function that inserts a node in the list can look the following way

int add_event( list_t *list, event_t *event )
{
    node_t **current = &list->head;

    while ( *current != NULL && !( strcmp( event->event_title, ( *current )->event.event_title ) < 0 ) )
    {
        current = &( *current )->next;
    }

    node_t *new_node = malloc( sizeof( node_t ) );
    int success = new_node != NULL;

    if ( success )
    {
        new_node->event = *event;
        new_node->next = *current;

        *current = new_node;
    }

    return success;
}

In main you just declare a list like

list_t list = { .head = NULL };

When you can write a function that reads data from a file, fills the structure event and calls the function add_event passing a pointer to the list and a pointer to the filled structure.

Here is a simplified demonstrative program.

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

//struct for event time
typedef struct{

  int hour;
  int minute;

} event_time_t;

//struct for event date
typedef struct{

  int month;
  int day;
  int year;

} event_date_t;

//struct for all event info
typedef struct{
  char event_title[20];
  event_time_t event_time;
  event_date_t event_date;
} event_t;

//struct for node of the list
typedef struct node
{
    event_t event;
    struct node *next;
} node_t;

//struct for the list itself
typedef struct
{
    node_t *head;
} list_t;

int add_event( list_t *list, event_t *event )
{
    node_t **current = &list->head;

    while ( *current != NULL && !( strcmp( event->event_title, ( *current )->event.event_title ) < 0 ) )
    {
        current = &( *current )->next;
    }

    node_t *new_node = malloc( sizeof( node_t ) );
    int success = new_node != NULL;

    if ( success )
    {
        new_node->event = *event;
        new_node->next = *current;

        *current = new_node;
    }

    return success;
}

void print_list( const list_t *list )
{
    for ( const node_t *current = list->head; current != NULL; current = current->next )
    {
        printf( "%s -> ", current->event.event_title );
    }
    puts( "null" );
}

int main(void) 
{
    list_t list = { .head = NULL };

    event_t event = { "third", { 0 }, { 0 } };

    add_event( &list, &event );

    print_list( &list );

    strcpy( event.event_title, "first" );

    add_event( &list, &event );

    print_list( &list );

    strcpy( event.event_title, "second" );

    add_event( &list, &event );

    print_list( &list );

    return 0;
}

The program output is

third -> null
first -> third -> null
first -> second -> third -> null

This is a starting point for developing your project.

Sorting Linked List Alphabetically while new nodes are being scanned in (C)

1 Answers1