tags:

views:

80

answers:

3

I have made a program which is a small library operated via software. When I add two books and then delete the first book the second book gets the same bookid as the first book because of count-- in the del() function. I cannot rely on printing the count as the bookid. Is there a better option?

#include<stdio.h>
#include<conio.h>
#include<stdlib.h>
static int count;
struct book
{
  int bookid;
  char name[30];
  char author[30];
  float price;
};
struct book b[40];
void add(void);
void del(void);
void sort(void);
void price(void);
void print(void);
void main(void)
{
  char choice;
  while(1)
  {
    clrscr();
    printf("Enter a choice:\n 1.Add a book.\n 2.Delete a book.\n 3.Sort books by price.\n 4.To print all books details.\n 5.To print the names of the books whose price is less than 1000.\n 6.Exit\n");
    choice=getche();//doing by getch() as getche makes the program rough as it is printed
    switch(choice)
    {
      case'1':add();break;
      case'2':del();break;
      case'3':sort();break;
      case'4':print();break;
      case'5':price();break;
      case'6':exit(0);
      default:printf("Enter a valid choice.");break;
    }
  }/*switch ends*/
}
void add(void)
{
  int i;
  char ch[30];
  clrscr();
  for(i=count;i<40;i++)
  {
    printf("Enter books name:\n");
    gets(b[i].name);
    printf("Enter author's name\n");
    gets(b[i].author);
    printf("Enter price:\n");
    gets(ch);
    b[i].price=atoi(ch);
    printf("Dear User,the book has succesfully been added.The book id is %d",i);
    count++;
    break;
  } /* for ends*/
  getch();
}
void print(void)
{
  int i;
  clrscr();
  for(i=0;i<count;i++)
  {
    printf("Bookid=%d,Name=%s,Author=%s,Price=%f\n",b[i].bookid,b[i].name,b[i].author,b[i].price);
  }
  getch();
}

void del(void)
{
  int i,j;
  char ch[10];
  clrscr();
  printf("Enter book id:");
  gets(ch); // how do i put it into the structure as i dont know that which structure it belongs to
  for(i=0;i<count;i++)  //searching
  {
    if(b[i].bookid==atoi(ch))
    {
      for(j=i;j<count;j++)
      {
        b[j]=b[j+1];
      }//for j ends
    }  //if ends
  } /* for of i ends */
  count--;
  getch();
}
//void del(void)
//{

    // int i;
    // char ch[10];
     // clrscr();
 //printf("Enter book id:");
       // gets(ch);
      // for(i=0;i<40;i++)
      // {
     //  b[i]=b[i+1];
    //
   // }
    // count--;
  // printf("Dear user,delete succesful");
//getch();
//}
void sort(void)
{
  int i;
  float temp;
  for(i=0;i<40;i++)
  {
    if(b[i].price>b[i+1].price)
    {
      temp=b[i].price;
      b[i].price=b[i+1].price;
      b[i+1].price=temp;
    }
  }/*for ends*/
  printf("Dear user,the books are sorted by price.\n");

  getch();
}

void price(void)
{
  int i;
  clrscr();
  for(i=0;i<count;i++)
  {
    if(b[i].price<1000)
    {
      printf("%d.%s\n",i+1,b[i].name);
    }
  }
  getch();
}
A: 

some pointers:

use fgets instead of gets, its safer because you can specify max buf len
in add() there is no book id assigned, so it will - due to being global - remain 0
why do u need book id? you have an array of 40, use the array index as id.

Anders K.
my teacher dint told me abt fgets but ppl here told me abt it.i tried using fgets by just adding f to gets but the parameters were missing..if i use array index as id and if i delete a book from between then ? whose going to fill that hole?
fahad
b[i].bookid=count;
fahad
@fahad It's not as simple as adding an `f` to `gets`. The reason *why* `fgets` is safer is because it takes an extra parameter describing the maximum length of the input, so you have to provide that parameter. Documentation on `fgets` (for documentation on most C functions, just google for "man <function name>"): http://linux.die.net/man/3/fgets
Tyler McHenry
A: 

Your first problem seems to be that you are never actually setting the bookid field of the book structure to anything. It will end up having some arbitrary value in each book struct, which will make it pure luck if del ever works correctly.

Your problem is in here:

printf("Enter books name:\n");
gets(b[i].name);
printf("Enter author's name\n");
gets(b[i].author);
printf("Enter price:\n");
gets(ch);
b[i].price=atoi(ch);
printf("Dear User,the book has succesfully been added.The book id is %d",i);
count++;

Take a look at this code, and show me where you set the value of b[i].bookid. The answer is nowhere. So try changing the last few lines to:

b[i].bookid = i;
printf("Dear User,the book has succesfully been added.The book id is %d", b[i].bookid);
count++;

Now, this still has a problem if you ever call add after calling del, because in the single-iteration loop that that code is in, i is always set to count. So, as you noticed, if count ever changes, which it will frequently, you will assign duplicate IDs. One solution is to define, at the top of the add function, a static variable (which retains its value between calls to the function) indicating what the next book id should be, like so:

void add(void)
{
  static int nextBookId = 0;
  int i = count;
  char ch[30];

  /* Do not overrun the array */
  if (count >= 40) return;

  clrscr();
  printf("Enter books name:\n");
  gets(b[i].name);
  printf("Enter author's name\n");
  gets(b[i].author);
  printf("Enter price:\n");
  gets(ch);
  b[i].price=atoi(ch);

  /* Assign the next unique book ID to this book, then increment nextBookId,
     which will retain its incremented value next time you call add() */
  b[i].bookid = nextBookId++;

  printf("Dear User,the book has succesfully been added.The book id is %d", b[i].bookid);
  count++;
  getch();
}

Note that I replaced your loop with a simple bounds check at the top.

This way, every book you add gets its own, unique identifier that does not necessarily correspond to its position in the array or the number of books that existed at the time it was added. This method will not re-use IDs of previously deleted books, but that is probably a desirable behavior.

Tyler McHenry
thanks a lot! it really gave me some brain:)
fahad
+1  A: 
Paul E.