Solved

Linked list again

Posted on 2000-04-28
41
198 Views
Last Modified: 2013-12-14
Hi,

I put together some code to insert a link in a linked list but a few errors keep on cropping up which i just can't understand. Could you please have a look and tell me why this is happening??

I'm in a bit of a hurry so a quick response would be grateful
Thanks

Heres the structre
struct link_list
{
      int empno;
      char empname  [20];
      float salary;
      int age;
      struct link_list   *next;
};typedef  struct  link_list  node;

node  *insert (node *list , int  no)
{
node *p1, *p2, p;
char name [20];
int ag,flag=0;
float sal;
p1 = list;
p2 = list;

for (;p2!=NULL;p2 = p2->next)
{
      p1 = p2;
      if (p2->empno == no)
      {
            flag = 1;
            printf ("\nYou  can't enter duplication employee number\n");
            break;
      }
}
      if (flag == 0)
      {
      p = (node *) malloc(sizeof(node));   /*  Cannot convert link_list* to link_list */
      printf ("Enter name , salary, age\n");
fflush (stdin);
        scanf ("%s   %f  %d",name,&sal,&ag);
        p->empno=no;         /* OK */
        strcpy (p->empname,name);    /* Pointer to structure */
        p->salary = sal;           /*   required on left side -> or ->*  */
        p->age = ag;
        p->next = NULL;
        p1->next = p;     /*  Cannot convet link_list to
                          link_list*   */
      }
return (list);
}

function called as
 list = insert (list , p);
0
Comment
Question by:anita177
  • 25
  • 16
41 Comments
 

Author Comment

by:anita177
ID: 2758828

Note the comments on the side are specifically where the compiler says the error is occuring
0
 
LVL 22

Accepted Solution

by:
nietod earned 200 total points
ID: 2758844
The main problem is you declared "p" as a node, not as a pointer to a node.

continues
0
 
LVL 22

Expert Comment

by:nietod
ID: 2758853
you have

node *p1, *p2, p;

which should be

node *p1, *p2, *p;

But, I woudl recommend you never us this type of declaration, where multiple variables are defined in one statement.  it can lead to problems lit this and others.  (There is nothing enhierently wrong with it, its just that it is much more likely to be miss-read, as you've done here)  Just define a just one variable in statement at a time, like

node *p1;
node  *p2
node  *p;

This is easier to read, and easier to change.

continues
0
 
LVL 22

Expert Comment

by:nietod
ID: 2758861
Another think I don't like is that you are using malloc() and free() in your programs.  Those are fine for C  (actually they are horrible in C, but there is no other choice in C).  In C++ however they should never be used (with one small exception.)  You should be using new and delete instead.  They are much easier to use and there is little chance of making a mistake with them.  Well a lot less of a chance.  They also do things that malloc() and free() don't, like construct and destruct objjects.

So replace

p = (node *) malloc(sizeof(node));  

with

p = new node;

to delete the node you would do

delete p;

instead of

free(p);

new and delete are covered in any C++ text book.  I recommend you look them up.  It is well worth it.
0
 

Author Comment

by:anita177
ID: 2758908
thanks for that.. i'm going to change the malloc() to new .. i'll get back to you if i have any other probelms
thanks again
0
 
LVL 22

Expert Comment

by:nietod
ID: 2758971
you must also change free to delete.  i.e. malloc() and free() work together as a pair and new and delete work together as a pair.  You can't mix malloc and delete or new and free(). (you can mix them in a program, but for a particular block of memory, if it is allcoated with one, it must be deleted with the cooresponding one in the pair.)
0
 

Author Comment

by:anita177
ID: 2760107

I'm back again.. needing help as usual!
i've been tracing through this bit of code and it dosen't seem to enter th while statement (check if nothing more is to be added)
Could you please have a look as it has been bugging me for ages...

node  *create (node *list)
{
       char fsource[20],p[6],fdestn[20];
      float ffare;
      node *insert (node *, char *) ;

      printf ("Enter a Flight code :\n");
      printf ("Enter  end to finish   \n");

      fflush (stdin);
      scanf ("%s",p);
      while (strcmp(p,"end") == 0)/*  this is the one that dosen't work */
      {

            if (list == NULL)
            {
                  list= new node;
                  strcpy(list->flight_code,p);
                  printf ("Enter Flight source, destination and fare\n");
                  fflush (stdin);
                  scanf ("%s %s %s",fsource, fdestn, &ffare);
                  strcpy (list->source,fsource);
                    strcpy (list->destn,fdestn);
                  list->fare = ffare;

                  list->next=NULL;
            }
            else
            {
                  list = insert (list,p);
            }
            printf ("Enter Flight code:  \n");
            scanf ("%s",p);
      }
return (list);
}

0
 
LVL 22

Expert Comment

by:nietod
ID: 2760236
You probably want it to be "!= 0"  strcmp() returns 0 when the strings are the same.  You want to run the loop as long as the strigns are different, that is as long as strcmp() does not return 0.
0
 

Author Comment

by:anita177
ID: 2760279
oh, i can't believe i didn't see that.........thanks alot
0
 

Author Comment

by:anita177
ID: 2761965
Hello,
following on from the project yesterday
I'm trying to save the linked list to a text file. The save function is only called once and the head is passed to it

save(head)
Can u tell me what's wrong with it as it dosen't seem to write to the file.


void save(node *list)
{
     FILE *fp;
     if((fp = fopen("flights.txt","wb")) == NULL)
     {
           printf("\n Cannot open FIle ");
      exit(1);
     }
        while(list != NULL )/* doesn't enter loop */
     {
            fwrite(list,sizeof(struct link_list),1,fp);
                  list = list->next;

Thanks
     }

     fclose(fp);
}
0
 

Author Comment

by:anita177
ID: 2762015
please hurry
0
 
LVL 22

Expert Comment

by:nietod
ID: 2762034
Well the "thanks" would generate a compiler error...

I don't see anything wrong with it.

Does a file with that name get created?   The file should be created in the same directory as the program you are running.

Does the while loop run?
0
 

Author Comment

by:anita177
ID: 2762044
no the "thanks" wasn't a problem i just put that in at the wrong place..
i checked and found a flights.txt. it does contain the linked list!!! it kept on exiting because my menu option was wrong for save()...
Thanks again.. i probably wouldn't have checked for a flights file without your help
 
0
 

Author Comment

by:anita177
ID: 2762151
hi,

Just one last thing... i need to load the text file into a linked list when the program starts up.
I’ve got this far, but i don’t think it is linking properly.

load is called as
at the start of the program
head = load (head);

head is NULL at the start.  The function should return the whole linked list so as other functions can use the values aswell.

node *load(node *newlist)
{
   /*node *list;*/
   node *p1;
       FILE *fp;

   p1 = newlist;

   fp = fopen("mlist.txt","r+");

   fflush(stdin);

   while (!(feof(fp)))
   {
         if(fread(newlist,sizeof(struct link_list),1,fp) != 1);
                                   break;

     newlist->next = new node;
   }
      if(!newlist->next)
      {
            printf("\n Out of memory ");
         return NULL;
      }
      newlist->next = NULL;
              p1->next = newlist;
      return(newlist);
   fclose(fp);
}




Thanks
0
 

Author Comment

by:anita177
ID: 2762155
Adjusted points from 50 to 80
0
 

Author Comment

by:anita177
ID: 2762341
please hurry, i'm still waiting for a reply
0
 
LVL 22

Expert Comment

by:nietod
ID: 2762397
That has a bunch of problems.  The first problem is how is it suposed to return the new list to the caller?   i.e. the caller has a pointer to the first item in the list, that pointer needs to be updated, but you have't provided any means to do so.  So you can read new items into memory, but you can't "tell" the caller where they are.

A 2nd problem is that your code will place an empty/invalid node at the end of the list it does create.  This is because each time you read an itme, you then create a new uninitialized node and store it as the next item after the item read.  So you always have one extra node in the list.  When the last item is read, there will still be one extra node in the list.

continues
0
 
LVL 22

Expert Comment

by:nietod
ID: 2762400
To solve the first problem, you could have the procedure return a pointer to the start of a new list, like

node *load()
{
   node *NewList = NULL;

  // create list.

  return NewList;
}

This procedure would return NULL if the new list is empty, or a pointer to the first item in the new list.

Or you could have the procedure take a reference to the caller's pointer to the start of the list.  Then it could alter the caller's pointer, so the caller would then have a pointer to the start of the new list, like

load(node *&ListHead)
{
    // change ListHead if needed, use it as needed.
};

The first approach would be best if the code should always create a brand new list from what it reads.  The 2nd approach is best if the code should add what it reads to an existing list.  So for, example, if there are already 2 entries in the list, the 2nd example might add new items before or after those 2 items.    

continues
0
 
LVL 22

Expert Comment

by:nietod
ID: 2762416
I'll go with the 1st option in showing you how to solve the 2nd problem.  althogu this siolution could be applied to either.  The idea is you need to keep a pointer to the start of the list and also a pointer to the end of the list, so you can add to the end of the list (creating a new end) but without loosing the pointer to the start of the list.  This allows you to add items to the list only when needed, not before it is needed

// return -> to start of new list.  NULL for an empty list.
node *load(
{
   node *Head = NULL
   node *Tail = NULL;
   FILE *fp;


   while (!(feof(fp)))
   {
       node *Current = new node;

       if (Tail) // If there already is a list started, then
          Tail->next = Current;  // Add new item to end of list.
       else // Otherwise if this was an empty list.
          Head = Current; // Save as first item.
       Tail = Current; // New item is the new tail.
       fread(Current,sizeof(link_list),1,fp);
    }
    fclose(fp);
    return Head;
}
0
 

Author Comment

by:anita177
ID: 2762430
thanks i'll give it a try
0
How to improve team productivity

Quip adds documents, spreadsheets, and tasklists to your Slack experience
- Elevate ideas to Quip docs
- Share Quip docs in Slack
- Get notified of changes to your docs
- Available on iOS/Android/Desktop/Web
- Online/Offline

 

Author Comment

by:anita177
ID: 2763521
Hi,

I still have a problem with the program.
The save function is fine. it does maage to store verything properly, but when it reads the data to the linked list it only reads the first record properly(even if there are more than 1 stored in the file)..
When i try to print the values after load() i see only the first record is ok the others contain garbage values and then it just stops.
error says "thread stopped"
Why can this be happening???

0
 
LVL 22

Expert Comment

by:nietod
ID: 2763540
Opps, after

      fread(Current,sizeof(link_list),1,fp);

you need

     Current->next = NULL;

but that might not be _the_ problem.  It is more likely you are printing the list incorrectly, or storing the list inorrectly after it is loaded.
0
 

Author Comment

by:anita177
ID: 2763552
i've added the line after fread()
it still stops at print though...
I'm sure it's not a problem with the print function but i'll show you what i have

void print (node *list)
{
      node *k;

      printf ("\nflight code | source | destn | arrtime | deptime  | seats | ");
   printf (" fare | date");
   printf("\n--------------------------------------------------------------\n");

      for (k=list;  k!= NULL;k= k->next)
      {printf ("\n  %s     %s    %s     %s     %s       %d     %.2f     %02d %02d %02d",
               k->flight_code,k->source,k->destn,
                        k->arrtime,k->deptime,k->seats,
                        k->fare,k->date.day,k->date.month,k->date.year);
      }

it has worked before!1
0
 
LVL 22

Expert Comment

by:nietod
ID: 2763575
Are these strings being stored directly in the node, or are you storing pointers to the string, like

struct node
{
    char Flight_code[10];
};

or

struct node
{
   char *Flight_code;
};

(If you are storing pointer in the node (other than next) it is a problem.)
0
 

Author Comment

by:anita177
ID: 2763584
no they are just stored
as

char flight_code[6] e.t.c
0
 

Author Comment

by:anita177
ID: 2763603
........no they are just stored
as

char flight_code[6] e.t.c

0
 
LVL 22

Expert Comment

by:nietod
ID: 2763645
how are you using load()?
0
 

Author Comment

by:anita177
ID: 2763649
ok, when the program starts up the first option a user enters is to load() from txt file..

so now the list should be created and new items can be added with the insert function which i already have... does this not sound right..??
0
 
LVL 22

Expert Comment

by:nietod
ID: 2763684
I meant programatically.

You must have some pointer somewhere that is a pointer to the start of the list.  This pointer would be set by Load().  Like

node *ListHead;

   *   *    *

ListHead = Load();
0
 

Author Comment

by:anita177
ID: 2763695
oh, sorry when it is called it is

head = load();

and load returns a pointer to first node

node *load()
{
######

return head;
}
0
 
LVL 22

Expert Comment

by:nietod
ID: 2763705
why don't you e-mail me the whole program.  nietod@journey.com.
0
 

Author Comment

by:anita177
ID: 2763716
Thanks,

I will
0
 

Author Comment

by:anita177
ID: 2765964
Hi just one last last time.... it’s got to be handed in tommorrow so i won’t bug you after this..
I’m trying to put togehter this bookings program.. the loading is ok but it seems to print found records
twice .. why is this??


// program to book seats on flights
// booking can be done by either giving destination
// or date of travel
// flights.txt flie is searched for the required flight
//if found another proc() called (not done yet)
void booking(void)
{

            int choice;
      char bdestn[20],bflcode[6];
      node *list;
      node *p1;
      node *finddestn(node *,char *);
      void printrecord(node *);
      FILE *fp;
      clrscr();

// load into linked list so a search can be perfored
      list = load();
         if((fp = fopen("flights.txt","r+")) == NULL)
         {
                 printf("\n Cannot open FIle ");
            exit(1);
         }

           printf ("\n 1. Check availability on destination ");
      printf ("\n 2. Check availability on date \n");

      fflush(stdin);
      scanf("%d",&choice);

      switch(choice)
      {
            case 1:
             printf("\n\n Enter desired destination : ");
          fflush(stdin);
          gets(bdestn);

                   if (strcmp(list->destn,bdestn) == 0)
                        printrecord(list);
                 else
                 {
                           p1 = finddestn(list,bdestn);
                         if (p1 == NULL)
                                        printf ("\nNo more matches\n");
                              else
                     printrecord(p1);
                  }
       }
       // decide which one of the flights listed is most suitable for
       // passenger, then book it
       printf("\n Enter flight code of the flight you have chosen to book");
       gets(bflcode);
     //  if (bflcode->seats == 0)  // need the pointer instead of bflcode
             //      printf("\n\n Sorry Flight is already fully booked !");
      // else
                   /*  proc book seat */
              break;
                     case 2:
                        printf("\n Not implemented yet ");
      }
}




// function to search for valid flights.. a list of all flights
//containing the key (destn) is to outputted printrecord()
//help it doesn't work!!

node *finddestn(node *list,char *key)
{

   void printrecord(node *);

      if (strcmp(list->next->destn,key) == 0)
         printrecord(list->next);
      else
      {
         if (list->next->next == NULL)
      {
            //      printf("\n\n reached null \n");
            return(NULL);
      }
            else
                  finddestn (list->next,key);
      }
}


0
 

Author Comment

by:anita177
ID: 2765969
Adjusted points from 80 to 150
0
 

Author Comment

by:anita177
ID: 2766023
R u there today???
0
 
LVL 22

Expert Comment

by:nietod
ID: 2766642
The logic there is a total mess.  Your

node *finddestn(node *list,char *key)

function oeprates recusively for no good reason.  Again this is what I've been warning you about, you have a fucntion that calls a function that calls a function, that calls....

Soemtimes that is a good idea, if you can be sure that the number of calls will be a reasonable minmimum.  Here the number of calls depends on the number of items in the list, so the more items, the more calls, you don't have much control over that, so it coudl be too many calls.  Probably not, but could be.  More importantly, there is no reason for this approach.  why not just use a loop that starts at the beginning of the list and tests each item in the list, if the item should be printed, it prints it.

I strongly suspect this double printing will go a way with a simpler design (the reason you are getting it is the print code then calls back to finddestn().)
0
 

Author Comment

by:anita177
ID: 2766715
ok, simple.. something along these lines

void finddestn(node * list,char *bdestn)
{
     while(list != NULL)
     {
       if(strcmp(list->destn,bdestn) == 0)

        printrecord(list);

       else
          list = list->next
      }
}

is that ok?? the thing i'm worried about is that when the list gets passed what if it misses out nodes earlier than when it was passed e.g. the start node.. will this function check the complete list or only from then onwards from the one it is passed??
i hope you can understand what i'm saying...


0
 

Author Comment

by:anita177
ID: 2766724
strcmp !=0
0
 
LVL 22

Expert Comment

by:nietod
ID: 2766901
I don't understand your "worry part"  It might or might not be a reasonable concern, but I can't understand it.

Also the "else" is wrong.  you ALWAYS want to go to the next item in the list.  some itmes shoudl print, some items should not, but in either case you want to go to the next item, so it woudl be more like

void finddestn(node * list,char *bdestn)
{
   while(list != NULL)
   {
      if(strcmp(list->destn,bdestn) != 0)
         printrecord(list);
      list = list->next
   }
}
0
 

Author Comment

by:anita177
ID: 2802317
Adjusted points from 150 to 200
0
 

Author Comment

by:anita177
ID: 2802318
Thanks for everything, you have been a great help.(Extra points 4 u aswell)
0

Featured Post

Highfive Gives IT Their Time Back

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

Join & Write a Comment

Go is an acronym of golang, is a programming language developed Google in 2007. Go is a new language that is mostly in the C family, with significant input from Pascal/Modula/Oberon family. Hence Go arisen as low-level language with fast compilation…
Container Orchestration platforms empower organizations to scale their apps at an exceptional rate. This is the reason numerous innovation-driven companies are moving apps to an appropriated datacenter wide platform that empowers them to scale at a …
The viewer will learn how to use and create keystrokes in Netbeans IDE 8.0 for Windows.
The viewer will learn how to user default arguments when defining functions. This method of defining functions will be contrasted with the non-default-argument of defining functions.

757 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

20 Experts available now in Live!

Get 1:1 Help Now