Thread: Problem with library program's delete

This program uses linked lists to create a library of books. You organize the books using name, rating(1-5), and category((F)iction, (H)istory, (P)hilosophy and (A)rts).It allows you to add books, delete them, search for books, and print out the inventory. The problem I'm having is that there seems to be some problem deleting the books. Lets say you insert two books: The Rescue
Joseph Conrad
3
F
An Introduction to Philosophy
George Stuart Fullerton
5
P

Enter the book name to delete: An Introduction to Philosophy
Deleting book with name <The Rescue> from the bookstoreinventory.
Command?: p
My Inventory:


The Rescue
Joseph Conrad
3
F
So as you can see it ends up deleting the correct book but for somereason it says
Deleting book with name <*which ever book comes before that book>from the bookstore inventory,
And if there is no book that comes before it give out randomcharacters.


Here is my code:

Code:

                                                                                                                                                                   #include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
/**********************************************************************
// Linked List Definitions
//  Define your linked list node and pointer types
//  here for use throughout the file.
//
//
//   ADD STATEMENT(S) HERE*/
struct Book {
char *name;
char *author;
int rating;
char category;
struct Book *next;
};

/**********************************************************************
// Linked List Function Declarations
//
// Functions that modify the linked list.
//   Declare your linked list functions here.
//
//   ADD STATEMENT(S) HERE*/
struct Book *newBook();
int bookExists(struct Book *bookHead, char bookName[]);
struct Book *insertBook(struct Book *bookHead, struct Book *book);
int compareBook(struct Book *book1, struct Book *book2);
struct Book *deleteBook(struct Book *bookHead, char bookName[]);
void searchBook(struct Book *bookHead, char bookName[]);
void searchBookRatingsHigherThan(struct Book *bookHead, int rating);
void printBook(struct Book *book);
void printBooks(struct Book *bookHead);
struct Book *deleteBooks(struct Book *bookHead);
/*********************************************************************
// Support Function Declarations
/*/
void safegets (char s[], int arraySize);     /*gets without buffer overflow*/
void bookNameDuplicate (char bookName[]);   /*marker/tester friendly*/
void bookNameFound (char bookName[]);       /*functions to print*/
void bookNameNotFound (char bookName[]);    /*messages to user*/
void bookNameDeleted (char bookName[]);

void printInventoryEmpty (void);
void printInventoryTitle (void);
void printNoBooksWithHigherRating (int rating);
/**********************************************************************
// Program-wide Constants
//*/
const int MAX_LENGTH = 1023;
const char NULL_CHAR = '\0';
const char NEWLINE = '\n';
const char FICTION = 'F';
const char HISTORY = 'H';
const char PHILOSOPHY = 'P';
const char ART = 'A';

/**********************************************************************
// Main Program
/*/
int main (void)
{
const char bannerString[]
    = "Online Bookstore Inventory Management Program.\n";
const char commandList[]
    = "Commands are I (insert), D (delete), S (search by name),\n"
      "  V (search by rating), P (print), Q (quit).\n";
/* Declare linked list head.
//   ADD STATEMENT(S) HERE TO DECLARE LINKED LIST HEAD.*/
struct Book *bookHead = NULL;
/* announce start of program*/
printf("%s", bannerString);
printf("%s", commandList);
char response;
char input[MAX_LENGTH+1];
do
{
    printf("\nCommand?: ");
    safegets(input, MAX_LENGTH + 1);
    // Response is first char entered by user.
    // Convert to uppercase to simplify later comparisons.
    response = toupper(input[0]);
    if (response == 'I')
    {
        // Insert a book entry into the linked list.
        // Maintain the list in correct order (F, H, P, A).
        //   ADD STATEMENT(S) HERE
        struct Book *book = newBook();
        // USE THE FOLLOWING PRINTF STATEMENTS WHEN PROMPTING FOR DATA:
        printf("  book: ");
        safegets(book->name, 255);
        printf("  author: ");
        safegets(book->author, 255);
        printf("  rating: ");
        book->rating = getchar() - '0';
        fflush(stdin);
        printf("  category: ");
        book->category = getchar();
        bookHead = insertBook(bookHead, book);
        fflush(stdin);
    }
    else if (response == 'D')
    {
        // Delete a book from the list.
        printf("Enter the book name to delete: ");
        //   ADD STATEMENT(S) HERE
        char bookName[10000];
        safegets(bookName, 10000);
        deleteBook(bookHead, bookName);
    }
    else if (response == 'S')
    {
        // Search for a book by a name.
        printf("Enter the book name to search for: ");
        //   ADD STATEMENT(S) HERE
        char bookName[10000];
        safegets(bookName, 10000);
        searchBook(bookHead, bookName);
    }
    else if (response == 'V')
    {
        /* Search for books that are rated higher than or equal to a rating.*/
        printf("Enter rating: ");
        //   ADD STATEMENT(S) HERE
        int rating = 0;
        scanf("%d", &rating);
        searchBookRatingsHigherThan(bookHead, rating);
        fflush(stdin);
    }
    else if (response == 'P')
    {
        // Print the bookstore inventory.
        //   ADD STATEMENT(S) HERE
        printBooks(bookHead);
    }
    else if (response == 'Q')
    {
        printf("\nThe book store inventory is empty"); /* do nothing, we'll catch this case below*/
        return 0;
    }
    else
    {
        // do this if no command matched ...
        printf("\nInvalid command.\n%s\n", commandList);
    }
} while (response != 'Q');
// Delete the entire linked list that hold the bookstore invento
//   ADD STATEMENT(S) HERE
bookHead = deleteBooks(bookHead);
// Print the linked list to confirm deletion.
//   ADD STATEMENT(S) HERE
printBooks(bookHead);
return 0;
}
/***********************************************************************
Support Function Definitions*/
/* Function to get a line of input without overflowing target char array*/
void safegets (char s[], int arraySize)
{
int i = 0, maxIndex = arraySize - 1;
char c;
while (i < maxIndex && (c = getchar()) != NEWLINE)
{
    s[i] = c;
    i = i + 1;
}
s[i] = NULL_CHAR;
}
/* Function to call when a user is trying to insert a book name
// that is already in the inventory.*/
void bookNameDuplicate (char bookName[])
{
printf("An entry for <%s> is already in the bookstore inventory!\n", bookName);
printf("New entry not entered.\n");
}
/* Function to call when a book with this name was found in the inventory.*/
void bookNameFound (char bookName[])
{
printf("The book with name <%s> was found in the bookstore inventory.\n",bookName);
}
/* Function to call when a book with this name was not found in the inventory.*/
void bookNameNotFound (char bookName[])
{
printf("The book with name <%s> is not in the bookstore inventory.\n", bookName);
}
// Function to call when a name that is to be deleted
// was found in the inventory.
void bookNameDeleted (char bookName[])
{
printf("Deleting book with name <%s> from the bookstore inventory.\n", bookName);
}
// Function to call when printing an empty bookstore inventory.
void printInventoryEmpty (void)
{
printf("The bookstore inventory is empty.\n");
}
// Function to call to print title when the entire inventory is printed.
void printInventoryTitle (void)
{
printf("My Inventory: \n");
}
/* Function to call when no book in the bookstore inventory has higher*/
// or equal rating to the given rating
void printNoBooksWithHigherRating (int rating)
{
printf("No book(s) in the bookstore inventory is rated higher than or equal to <%d>.\n", rating);
}
//**********************************************************************
// Add your functions below this line.
// Allocates a memory space for the book
struct Book *newBook()
{
struct Book *book = (struct Book *)malloc(sizeof(struct Book));
book->author = malloc(255 * (sizeof(char)));
book->name = malloc(255 * (sizeof(char)));
book->rating = 0;
book->category = ' ';
book->next = NULL;
return book;
}
// Checks if the book already exists in the linked list
int bookExists(struct Book *bookHead, char bookName[])
{
if(bookHead == NULL)
    // return false
    return 0;
struct Book *bookCurrent = bookHead;
while(bookCurrent != NULL)
{
    if(strcmp(bookCurrent->name, bookName) == 0)
        // return true if match found
        return 1;
    bookCurrent = bookCurrent->next;
}
// return false
return 0;
}
// Compares two books and check if book 1 is more prioritized than book 2
int compareBook(struct Book *book1, struct Book *book2)
{
int book1Priority = 0;
int book2Priority = 0;
switch(book1->category)
{
    case 'F': book1Priority = 4; break;
    case 'H': book1Priority = 3; break;
    case 'P': book1Priority = 2; break;
    case 'A': book1Priority = 1; break;
}
switch(book2->category)
{
    case 'F': book2Priority = 4; break;
    case 'H': book2Priority = 3; break;
    case 'P': book2Priority = 2; break;
    case 'A': book2Priority = 1; break;
}
if(book1Priority > book2Priority)
    return 1;
if(book1Priority < book2Priority)
    return -1;
return 0;
}
// Inserts a new book into the linked list
struct Book *insertBook(struct Book *bookHead, struct Book *book)
{
if(bookExists(bookHead, book->name))
{
    bookNameDuplicate(book->name);
    return bookHead;
}
if(bookHead == NULL)
{
    // insert at the root node if empty
    bookHead = book;
    return bookHead;
}
// otherwise detect the special order F,H,P,A
// find the root of the category of the book
struct Book *bookPrevious = NULL;
struct Book *bookCurrent = bookHead;
while(bookCurrent != NULL)
{
    if(compareBook(book, bookCurrent) <= 0)
    {
        bookPrevious = bookCurrent;
        bookCurrent = bookCurrent->next;
        continue;
    }
    break;
}
if(bookPrevious == NULL)
{
    // if book previous is null, meaning we're inserting in the head
    book->next = bookHead;
    bookHead = book;
}
else
{
    // if not then we're inserting in either inbetween, or tail
    book->next = bookPrevious->next;
    bookPrevious->next = book;
}
return bookHead;
}
// Deletes a book from the linked list
struct Book *deleteBook(struct Book *bookHead, char bookName[])
{
// Check if the book exists
if(bookHead == NULL)
{
    printInventoryEmpty();
    return bookHead;
}
if(!bookExists(bookHead, bookName))
{
    bookNameNotFound(bookName);
    return bookHead;
}
// look for the book
struct Book *bookPrevious = NULL;
struct Book *bookCurrent = bookHead;
while(bookCurrent != NULL)
{
    if(strcmp(bookCurrent->name, bookName) == 0)
        break;
    bookPrevious = bookCurrent;
    bookCurrent = bookCurrent->next;
}
if(bookPrevious == NULL)
{
    // if the book previous is NULL that means we are deleting the head
    bookPrevious = bookHead;
    bookHead = bookHead->next;
    bookNameDeleted(bookPrevious->name);
    free(bookPrevious);
}
else
{
    // if we're not deleting the head, then we're deleting in between
    bookPrevious->next = bookCurrent->next;
    bookNameDeleted(bookPrevious->name);
    free(bookCurrent);
}
return bookHead;
}
// Searches a book from the linked list
void searchBook(struct Book *bookHead, char bookName[])
{
// Check if the book exists
if(bookHead == NULL)
{
    printInventoryEmpty();
    return;
}
if(!bookExists(bookHead, bookName))
{
    bookNameNotFound(bookName);
    return;
}
// look for the book and display its details
struct Book *bookCurrent = bookHead;
while(bookCurrent != NULL)
{
    if(strcmp(bookCurrent->name, bookName) == 0)
        break;
    bookCurrent = bookCurrent->next;
}
// display the book details
bookNameFound(bookCurrent->name);
printBook(bookCurrent);
}
// Searches a book that has higher rating than the given rating
void searchBookRatingsHigherThan(struct Book *bookHead, int rating)
{
if(bookHead == NULL)
{
    printInventoryEmpty();
    return;
}
// look for the book and display its details
int foundBooks = 0;
struct Book *bookCurrent = bookHead;
while(bookCurrent != NULL)
{
    if(bookCurrent->rating >= rating)
    {
        printBook(bookCurrent);
        foundBooks++;
    }
    bookCurrent = bookCurrent->next;
}
if(foundBooks == 0)
    printNoBooksWithHigherRating(rating);
}
// Displays book details
void printBook(struct Book *book)
{
printf("\n%s\n", book->name);
printf("%s\n", book->author);
printf("%d\n", book->rating);
printf("%c\n", book->category);
}
// Displays all books in the inventory
void printBooks(struct Book *bookHead)
{
if(bookHead!= NULL)
{
printInventoryTitle();
}
if(bookHead == NULL)
{
    printInventoryEmpty();
    return;
}
struct Book *bookCurrent = bookHead;
while(bookCurrent != NULL)
{
    printBook(bookCurrent);
    bookCurrent = bookCurrent->next;
}
}
// Delete all books from the memory
struct Book * deleteBooks(struct Book *bookHead)
{
if(bookHead == NULL)
    return bookHead;
struct Book *bookPrevious = NULL;
struct Book *bookCurrent = bookHead;
while(bookCurrent != NULL)
{
    bookPrevious = bookCurrent;
    bookCurrent = bookCurrent->next;
    free(bookPrevious);
}
if(bookPrevious != NULL)
    free(bookPrevious);
bookHead = NULL;
return bookHead;
}

Don't call fflush(stdin), it results in undefined behavior. Read this link: FAQ > Why fflush(stdin) is wrong - Cprogramming.com. Suggestions on workarounds here: FAQ > Flush the input buffer - Cprogramming.com.

As for your delete message being wrong, take a look at what you're printing:

Code:

if(bookPrevious == NULL)
{
    // if the book previous is NULL that means we are deleting the head
    bookPrevious = bookHead;
    bookHead = bookHead->next;
    bookNameDeleted(bookPrevious->name);
    free(bookPrevious);
}
else
{
    // if we're not deleting the head, then we're deleting in between
    bookPrevious->next = bookCurrent->next;
    bookNameDeleted(bookPrevious->name);
    free(bookCurrent);
}

Why would you expect it to print a message about the current book?

I just noticed you have a problem when deleting the first book from the list:

Code:

Command?: p
My Inventory:


resc
jo con
3
f


intro phil
geo full
5
p


Command?: d
Enter the book name to delete: resc
Deleting book with name <resc> from the bookstore inventory.


Command?: p
My Inventory:


(null)
jo con
3
f


intro phil
geo full
5
p

It still shows up in the list, albeit in a strange way. The memory has been freed, but the head of your list still points there. Accessing freed memory like that is undefined behavior. You need to update bookHead in main to contain the new head returned by bookDelete, similar to how you do with insertBook around line 101.

Also, you need to free the name and author fields before you free the book itself, otherwise you have a memory leak. Create a bookFree function, that does the opposite of your bookNew function.

If you are always going to malloc 255 bytes for the author and title, you should just use char arrays in your struct. But a better solution would be to only malloc the amount of memory you need. Read the author and title into temporary arrays, and pass those to bookNew. Then, you do something like:

Code:

struct Book *bookNew(char *name, char *author)
{
    struct Book *book = malloc(sizeof(*book));
    book->author = malloc(strlen(author) + 1);
    strcpy(book->author, author);
    // similar for name
    return book;
}

Notice how I used *book in the sizeof expression. book is a pointer, *book is the thing pointed to (a struct Book), so it is equivalent to what you have, but offers a slight advantage. If you ever change the type of book, you only need to change one place, the declaration. You can leave the malloc call alone. Also, notice that I did not cast the return value of malloc. It's usually recommended that you don't. Read this: FAQ > Casting malloc - Cprogramming.com

Originally Posted by anduril462

As for your delete message being wrong, take a look at what you're printing:

Code:

if(bookPrevious == NULL)
{
    // if the book previous is NULL that means we are deleting the head
    bookPrevious = bookHead;
    bookHead = bookHead->next;
    bookNameDeleted(bookPrevious->name);
    free(bookPrevious);
}
else
{
    // if we're not deleting the head, then we're deleting in between
    bookPrevious->next = bookCurrent->next;
    bookNameDeleted(bookPrevious->name);
    free(bookCurrent);
}

Why would you expect it to print a message about the current book?

Could you please tell me how I would have to change this code to make it print the current book?

Originally Posted by iMalc

The fact that you're asking that question says to me that, you haven't tried anything yet, and/or you didn't write that code yourself.
What sort of change do think think you need to make?

It actually is true that I did not write this code on my own and received a lot of help writing it from my friends since I am not familiar with linked lists. I have spent a lot of time trying to understand it and this the only problem which stands in the way of me completing this project.