• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 222
  • Last Modified:

Linked list again

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
anita177
Asked:
anita177
  • 25
  • 16
1 Solution
 
anita177Author Commented:

Note the comments on the side are specifically where the compiler says the error is occuring
0
 
nietodCommented:
The main problem is you declared "p" as a node, not as a pointer to a node.

continues
0
 
nietodCommented:
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
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
nietodCommented:
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
 
anita177Author Commented:
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
 
nietodCommented:
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
 
anita177Author Commented:

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
 
nietodCommented:
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
 
anita177Author Commented:
oh, i can't believe i didn't see that.........thanks alot
0
 
anita177Author Commented:
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
 
anita177Author Commented:
please hurry
0
 
nietodCommented:
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
 
anita177Author Commented:
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
 
anita177Author Commented:
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
 
anita177Author Commented:
Adjusted points from 50 to 80
0
 
anita177Author Commented:
please hurry, i'm still waiting for a reply
0
 
nietodCommented:
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
 
nietodCommented:
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
 
nietodCommented:
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
 
anita177Author Commented:
thanks i'll give it a try
0
 
anita177Author Commented:
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
 
nietodCommented:
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
 
anita177Author Commented:
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
 
nietodCommented:
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
 
anita177Author Commented:
no they are just stored
as

char flight_code[6] e.t.c
0
 
anita177Author Commented:
........no they are just stored
as

char flight_code[6] e.t.c

0
 
nietodCommented:
how are you using load()?
0
 
anita177Author Commented:
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
 
nietodCommented:
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
 
anita177Author Commented:
oh, sorry when it is called it is

head = load();

and load returns a pointer to first node

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

return head;
}
0
 
nietodCommented:
why don't you e-mail me the whole program.  nietod@journey.com.
0
 
anita177Author Commented:
Thanks,

I will
0
 
anita177Author Commented:
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
 
anita177Author Commented:
Adjusted points from 80 to 150
0
 
anita177Author Commented:
R u there today???
0
 
nietodCommented:
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
 
anita177Author Commented:
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
 
anita177Author Commented:
strcmp !=0
0
 
nietodCommented:
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
 
anita177Author Commented:
Adjusted points from 150 to 200
0
 
anita177Author Commented:
Thanks for everything, you have been a great help.(Extra points 4 u aswell)
0

Featured Post

VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

  • 25
  • 16
Tackle projects and never again get stuck behind a technical roadblock.
Join Now