user191776
user191776

Reputation:

Why does this C code print only the first & last node entered?

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

typedef struct 
{
  char *Name;
  int grade;
  int cost;
}Hotel;    /*This is data element in each node*/

typedef struct hinfo
{
  Hotel h;
  struct hinfo *next;
}Hinfo;   /*This is a single node*/

typedef struct
{
  Hinfo *next; /*This is the head pointer of the linked list*/
}HotelHead;

void createHotel(HotelHead *h);
void DisplayHotel(HotelHead h);

int main()
{
  HotelHead *list=(HotelHead *)malloc(sizeof(HotelHead));
  list->next=NULL;
  createHotel(list);
  DisplayHotel(*list);
  return(0);
}

void createHotel(HotelHead *h)  /*This function creates the list of hotels*/
{
  char ans='y';
  while(ans=='y' || ans=='Y')
  {
    char *name=(char *)malloc(20*sizeof(char));
    Hinfo *new=(Hinfo *)malloc(sizeof(Hinfo));
    printf("\nEnter hotel name: ");
    scanf("%[A-Za-z0-9 ]",name);
    printf("\nEnter hotel grade & cost: ");
    scanf("%d %d",&new->h.grade,&new->h.cost);
    new->h.Name=name;
    new->next=NULL;
    if(h->next==NULL){h->next=new;}
    else
    {
      Hinfo *current=h->next;
      while(current->next!=NULL){current->next=current->next->next;}
      current->next=new;
    }
    printf("\nEnter another hotel?(Y/N): ");
    scanf("%s",&ans);
    getchar();          /*dummy getchar to eat unwanted character*/
  }
}

void DisplayHotel(HotelHead h)  /*This function displays all hotels in the list*/
{
  Hinfo *current=h.next;
  printf("\nHotel list:\n");
  while(current!=NULL)
  {
    printf("\n%s %d %d\n",current->h.Name,current->h.grade,current->h.cost);
    current=current->next;
  }
}

Upvotes: 0

Views: 941

Answers (3)

Liz Albin
Liz Albin

Reputation: 1489

This is incorrect:

char *name=(char *)malloc(sizeof(20));

You are allocating (sizeof(int)) bytes, not 20 bytes.

No matter what else you're doing, this will cause problems.

Upvotes: 0

fmaste
fmaste

Reputation: 383

The DisplayHotel function is OK! The problem is with createhotel function. When you do:

while( current->next != NULL ){
  current->next = current->next->next;
}

Here you are actually changing the list, removing an element. Try doing:

while( current->next != NULL ){
  current = current->next;
}

Also the best approach would be to always have a pointer to the last element of the list on the head, so you add new elements directly instead of always going thorough the entire list! (remember to update the head when you add a new element)

Upvotes: 2

Michael Carman
Michael Carman

Reputation: 30831

You want to move current while walking the list instead of changing the value of current->next. Change this:

while (current->next != NULL) {
    current->next = current->next->next;
}

to this:

while (current->next != NULL) {
    current = current->next;
}

That said, it would be better to move current while adding new nodes instead of walking the linked list from the start every time. For example (skeleton code):

Hinfo *current;

while (...) {
    Hinfo *new = malloc(sizeof(Hinfo));

    // initialize new node

    if (current != NULL) {
        current->next = new;
    }

    current = new;

    // prompt to enter more nodes
}

Upvotes: 4

Related Questions