0

I am trying to write a method which returns book objects inside Student Objects linked list structure however while i am testing code works correct when i start deleting from the end however if i start deleting from beginning code does not deletes the last element i could not manage to fix the bug.

Here is my Structures below

class Student
{
public:
Student();
Student(int id,string name);
~Student();
Student(const Student& rightVal);
//Student& operator=(const Student& rightVal);
int getId();
void setId(int id);
string getName();
void setName(string name);
//studentBookNode methods
bool isEmpty() const;
int getLength() const;
bool removeNode(int index);
int findIndex(int studentId);
 struct studentBookNode
{
    Book item;
    studentBookNode* next;
};
studentBookNode* stuhead;
int studentBooks;
studentBookNode* findPointer(int index);
private:
int studentId;
string studentName;

};

class LibrarySystem
{
public:
LibrarySystem();
~LibrarySystem();
void addStudent(const int studentId, const string studentName);
void addBook(const int bookId, const string bookName, const int bookYear);
void checkoutBook(const int bookId, const int studentId);

void deleteBook(const int bookId);
void deleteStudent(const int studentId);
void returnBook(const int bookId);
void showAllBooks() const;
void showBook(const int bookId) const;
void showStudent(const int studentId) const;

bool isEmpty() const;
bool isBooksEmpty() const;
bool removeNode(int index);
bool removeBookNode(int index);

int findBookIndex(int bookId);
int findIndex(int studentId);
int getLengthBooks() const;
int getLength() const;

private:
struct Node
{
    Student item;
    Node* next;
};
Node* head;
int Size;
Node* findPointer(int index) const;
struct allBooksNode
{
    Book item;
    allBooksNode* next;
};
allBooksNode* bookHead;
int bookSize;
allBooksNode* findBookPointer(int index) const;

};

And here is my returnBook function:

void LibrarySystem::returnBook(const int bookId)
{
bool bookExist = false;
int bookIndex;
int stuIndex;
allBooksNode* curBook = bookHead;
int counter = 1;
for(int i = 1; i <= getLengthBooks();i++)
{
    if(curBook->item.getBookId() == bookId)
    {
        if(curBook->item.getStatus() == false)
        {
            bookIndex = i;
            bookExist = true;
        }
    }
    curBook = curBook->next;
}
if(bookExist)
{
    Student::studentBookNode* prev;
    curBook = findBookPointer(bookIndex);
    Student::studentBookNode* curSTN;
    for(Node* cur = head; cur != NULL;cur = cur->next)
    {

        for(Student::studentBookNode* temp = cur->item.stuhead; temp!= NULL;temp = temp->next)
        {


             if(temp->item.getBookId() == bookId)
            {
                if(cur->item.getLength() == 1)
                {
                    curSTN = cur->item.stuhead;
                    cur->item.stuhead = cur->item.stuhead->next;
                    cur->item.studentBooks--;
                    curSTN->next = NULL;
                    delete curSTN;
                    curSTN = NULL;
                     curBook->item.setStatus(true);
                cout<<"Book: "<<bookId<<" Has been returned"<<endl;

                }
                else
                {


                    prev = cur->item.findPointer(counter - 1);

                    curSTN = prev->next;

                    prev->next = curSTN->next;

                    cur->item.studentBooks--;

                     curSTN->next = NULL;

                    delete curSTN;
                    curSTN = NULL;
                     curBook->item.setStatus(true);
                cout<<"Book: "<<bookId<<" Has been returned"<<endl;


                }


            }


    counter++;

        }


    }



}
else
{
    cout<<"This student does not own Book:"<<bookId<<" To return"<<endl;
}



}

Here is my main method with some tests and outputs:

int main() {
LibrarySystem LS;
LS.addBook(1000, "Machine Learning", 1997);
LS.addBook(1200, "Data Mining", 1991);
LS.addBook(1300, "Problem S. with C++", 1991);
LS.addBook(1400, "C++ How to Program", 2005);
LS.addStudent(21000000, "blabla");
LS.addStudent(21000011, "zlalzla");
LS.checkoutBook(1400,21000011);
LS.checkoutBook(1300,21000011);
LS.checkoutBook(1200,21000011);
LS.returnBook(1400);
LS.returnBook(1200);
LS.returnBook(1300);
output is:
Book: 1400 Has been returned
Book: 1200 Has been returned

As it shown book for book 1300 function did not crashed however it did not returned the book either

Another test:

LS.returnBook(1200);
LS.returnBook(1300);
LS.returnBook(1400);
output:
Book: 1200 Has been returned
Book: 1300 Has been returned
Book: 1400 Has been returned

Now code works however i could not understand why this is happening?

  • 1
    Note: You have packed far too much responsibility into the `Student` class and this is making it harder for you to find and fix problems. `Student` should contain an instance of a `LinkedList` class rather than trying to be two different Linked List classes in addition to being a student. If you separate the responsibilities, you can test all of the linked list behaviour without `Student` getting in the way. You get a much, much simpler `Student` as a result and you get a `LinkedList` class you can keep using until the instructor finally gets around to covering Standard Library Containers. – user4581301 Dec 08 '20 at 19:54
  • In this linked list lass you will have methods for inserting, removing, and finding nodes. Returning a book becomes a call to find followed by a call to remove. Find and remove have one simple job each, making them easier to understand and test than one big function that tries to do everything. This is just more division of responsibilities. Most of the time you want simple code that does simple things that can be assembled like building blocks to make more complicated things. As soon as you have anything that does more than one thing, you're probably headed in the wrong direction. – user4581301 Dec 08 '20 at 19:59
  • @user4581301 thanks for your comments, however did you catch the problem? inside my function – saitcanbaskol Dec 08 '20 at 20:08
  • I didn't try to figure out what was wrong because what's there is extremely overcomplicated. If you don't reduce the complexity to something manageable you'll be picking bugs out for weeks. T he best thing you can do is put what you've written away for a while, break the problem down into smaller pieces, and attack the smaller pieces one-by-one. For example, A good find-and-remove function looks like the [community addition in this answer](https://stackoverflow.com/a/22122095/4581301). Ten lines of code, tops. – user4581301 Dec 08 '20 at 20:33

0 Answers0