-1

I am trying to import a book by prompting the user to provide its details in the command line one after another. For that I am using an overloaded input operator but when trying it out it results in a Segmentation fault. However, I was not able to find the root of the issue.

// main.cpp
#include "src/Buch.hpp"
#include "src/Bibliothek.hpp"
#include <iostream>
#include <limits>

void printMenu();

int main()
{

  Bibliothek bookList;

  int choice;
  do {
      printMenu();
      std::cin >> choice;
      std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); // Clear input buffer

      switch (choice) {
          case 1: {
              Buch newBook;
              std::cin >> newBook;
              bookList.insertBook(newBook);
              std::cout << "Book added successfully.\n";
              break;
          }
          case 2: {
              std::string ISBN;
              std::cout << "Enter ISBN: ";
              std::getline(std::cin, ISBN);
              Buch* foundBook = bookList.findBookByISBN(ISBN);
              if (foundBook != nullptr) {
                  std::cout << "Book found:\n" << *foundBook << std::endl;
              } else {
                  std::cout << "Book not found.\n";
              }
              break;
          }
          case 3: {
              std::string author;
              std::cout << "Enter author name: ";
              std::getline(std::cin, author);
              Bibliothek booksByAuthor = bookList.getBooksByAuthor(author);
              if (!booksByAuthor.empty()) {
                  std::cout << "Books by " << author << ":\n";
                  booksByAuthor.display();
              } else {
                  std::cout << "No books by " << author << " found.\n";
              }
              break;
          }
          case 4: {
              std::string filename;
              std::cout << "Enter file name to read from: ";
              std::getline(std::cin, filename);
              bookList.readFromFile(filename);
              break;
          }
          case 5: {
              std::string filename;
              std::cout << "Enter file name to save to: ";
              std::getline(std::cin, filename);
              bookList.writeToFile(filename);
              break;
          }
          case 6: {
              bookList.display();
              break;
          }
          case 7: {
              std::cout << "Exiting...\n";
              break;
          }
          default:
              std::cout << "Invalid choice. Please try again.\n";
      }
  } while (choice != 7);

  return 0;
}

void printMenu()
{
    std::cout << "===== Book Management System =====\n";
    std::cout << "1. Add a new book\n";
    std::cout << "2. Search for a book by ISBN\n";
    std::cout << "3. Search for books by author\n";
    std::cout << "4. Read book data from a file\n";
    std::cout << "5. Save book data to a file\n";
    std::cout << "6. Display all books\n";
    std::cout << "7. Exit\n";
    std::cout << "==================================\n";
    std::cout << "Enter your choice: ";
}

// Buch.hpp
#pragma once
#include <string>

class Buch
{
  private:
    std::string author;
    std::string title;
    std::string publisher;
    std::string isbn;
    double price;

  public:
    Buch();
    Buch(std::string author, std::string title, std::string publisher, std::string isbn, double price);
    Buch(const Buch& other);

    std::string getAuthor() const;
    std::string getTitle() const;
    std::string getPublisher() const;
    std::string getISBN() const;
    double getPrice() const;

    friend std::istream& operator>>(std::istream& is, Buch& buch);
    friend std::ostream& operator<<(std::ostream& os, const Buch& buch);
};

// Buch.cpp
#include "Buch.hpp"
#include <iostream>
#include <limits>
#include <ostream>
#include <string>

Buch::Buch() : author(""), title(""), publisher(""), isbn(""), price(0.0) {}

Buch::Buch(std::string author, std::string title, std::string publisher, std::string isbn, double price) : author(author), title(title), publisher(publisher), isbn(isbn), price(price) {}

Buch::Buch(const Buch& other) : author(other.author), title(other.title), publisher(other.publisher), isbn(other.isbn), price(other.price) {}

std::string Buch::getAuthor() const
{
  return author;
}

std::string Buch::getTitle() const
{
  return title;
}

std::string Buch::getPublisher() const
{
  return publisher;
}

std::string Buch::getISBN() const
{
  return isbn;
}

double Buch::getPrice() const
{
  return price;
}

// Input operator
std::istream& operator>>(std::istream& is, Buch& buch) {
    std::cout << "Enter author: ";
    std::getline(is, buch.author);
    std::cout << "Enter title: ";
    std::getline(is, buch.title);
    std::cout << "Enter Publisher: ";
    std::getline(is, buch.publisher);
    std::cout << "Enter ISBN: ";
    std::getline(is, buch.isbn);
    std::cout << "Enter price: ";
    is >> buch.price;
    // Consume the newline character
    is.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    return is;
}

// Output operator
std::ostream& operator<<(std::ostream& os, const Buch& buch) {
    os << "Author: " << buch.author << std::endl;
    os << "Title: " << buch.title << std::endl;
    os << "Publisher: " << buch.publisher << std::endl;
    os << "ISBN: " << buch.isbn << std::endl;
    os << "Price: $" << buch.price << std::endl;
    return os;
}

// Bibliothek.hpp
#pragma once
#include "Buch.hpp"

struct Buecherfach
{
  Buch data;
  Buecherfach* prev;
  Buecherfach* next;

  Buecherfach(Buch buch);
};

class Bibliothek
{
  private:
    Buecherfach* head;
    Buecherfach* tail;

  public:
    Bibliothek();

    void push_front(Buch& buch);
    void push_back(Buch& buch);
    Buch front() const;
    Buch back() const;
    void pop_front();
    void pop_back();

    void insertBook(const Buch& buch);
    Buch* findBookByISBN(const std::string& isbn) const;
    Bibliothek getBooksByAuthor(const std::string& author) const;

    bool empty() const;
    void display() const;

    friend std::istream& operator>>(std::istream& is, Bibliothek& list);    
    friend std::ostream& operator<<(std::ostream& os, const Bibliothek& list);

    void writeToFile(const std::string& filename) const;
    void readFromFile(const std::string& filename);
};

// Bibliothek.cpp
#include "Bibliothek.hpp"
#include <iostream>
#include <fstream>

Buecherfach::Buecherfach(Buch buch) : data(buch), prev(nullptr), next(nullptr) {}

Bibliothek::Bibliothek() : head(nullptr), tail(nullptr) {}

void Bibliothek::push_front(Buch& buch) {
    Buecherfach* neuesBuecherfach = new Buecherfach(buch);
    neuesBuecherfach->next = head;  // Set newNode's next pointer to current head
    neuesBuecherfach->prev = nullptr;  // Set newNode's previous pointer to nullptr since it's now the first node
    
    if (head != nullptr) {
        head->prev = neuesBuecherfach;  // Update the previous pointer of the current head to newNode
    }
    
    head = neuesBuecherfach;  // Update head to point to newNode
    if (tail == nullptr) {
        tail = neuesBuecherfach;  // If the list was empty, update tail to newNode
    }
}

void Bibliothek::push_back(Buch& buch) {
    Buecherfach* neuesBuecherfach = new Buecherfach(buch);
    neuesBuecherfach->next = nullptr;  // Since newNode is now the last node, its next pointer is nullptr
    neuesBuecherfach->prev = tail;  // Set newNode's previous pointer to current tail
    
    if (tail != nullptr) {
        tail->next = neuesBuecherfach;  // Update the next pointer of the current tail to newNode
    }
    
    tail = neuesBuecherfach;  // Update tail to point to newNode
    if (head == nullptr) {
        head = neuesBuecherfach;  // If the list was empty, update head to newNode
    }
}

Buch Bibliothek::front() const {
    if (head != nullptr) {
        return head->data;  // Return the value of the head node
    } else {
        throw "List is empty";  // If the list is empty, throw an exception
    }
}

Buch Bibliothek::back() const {
    if (tail != nullptr) {
        return tail->data;  // Return the value of the tail node
    } else {
        throw "List is empty";  // If the list is empty, throw an exception
    }
}

void Bibliothek::pop_front() {
    if (head == nullptr) {
        return;  // If the list is empty, do nothing
    }
    
    Buecherfach* temp = head;  // Store a temporary pointer to the current head
    head = head->next;  // Update head to point to the next node
    if (head != nullptr) {
        head->prev = nullptr;  // Update the previous pointer of the new head to nullptr
    } else {
        tail = nullptr;  // If the list becomes empty, update tail to nullptr
    }
    
    delete temp;  // Delete the node that was removed
}

void Bibliothek::pop_back() {
    if (tail == nullptr) {
        return;  // If the list is empty, do nothing
    }
    
    Buecherfach* temp = tail;  // Store a temporary pointer to the current tail
    tail = tail->prev;  // Update tail to point to the previous node
    if (tail != nullptr) {
        tail->next = nullptr;  // Update the next pointer of the new tail to nullptr
    } else {
        head = nullptr;  // If the list becomes empty, update head to nullptr
    }
    
    delete temp;  // Delete the node that was removed
}

void Bibliothek::insertBook(const Buch& buch) {
    Buecherfach* neuesBuecherfach = new Buecherfach(buch);
    if (head != nullptr) {
        head = tail = neuesBuecherfach;
        return;
    }

    Buecherfach* current = head;
    while (current) {
        if (current->data.getPrice() > buch.getPrice()) {
            if (current == head) {
                neuesBuecherfach->next = head;
                head->prev = neuesBuecherfach;
                head = neuesBuecherfach;
            } else {
                neuesBuecherfach->prev = current->prev;
                neuesBuecherfach->next = current;
                current->prev->next = neuesBuecherfach;
                current->prev = neuesBuecherfach;
            }
            return;
        }
        current = current->next;
    }

    tail->next = neuesBuecherfach;
    neuesBuecherfach->prev = tail;
    tail = neuesBuecherfach;
}

Buch* Bibliothek::findBookByISBN(const std::string& isbn) const {
    Buecherfach* current = head;
    while (current) {
        if (current->data.getISBN() == isbn) {
            return &(current->data);
        }
        current = current->next;
    }
    return nullptr; // Book with given ISBN not found
}

Bibliothek Bibliothek::getBooksByAuthor(const std::string& author) const {
  Bibliothek booksByAuthor;
  Buecherfach* current = head;
  while (current) {
      if (current->data.getAuthor() == author) {
          booksByAuthor.insertBook(current->data);
      }
      current = current->next;
  }
  return booksByAuthor;
}

bool Bibliothek::empty() const {
    return head == nullptr;
}

// Function to display all books in the list
void Bibliothek::display() const {
    Buecherfach* current = head;
    while (current) {
        std::cout << current->data << std::endl;
        current = current->next;
    }
}

void Bibliothek::writeToFile(const std::string& filename) const {
    std::ofstream outFile(filename);
    if (outFile.is_open()) {
        Buecherfach* current = head;
        while (current) {
            outFile << current->data << std::endl;
            outFile << std::endl; // Leave an empty line
            current = current->next;
        }
        outFile.close();
        std::cout << "Data written to file successfully.\n";
    } else {
        std::cerr << "Unable to open file: " << filename << std::endl;
    }
}

void Bibliothek::readFromFile(const std::string& filename) {
    std::ifstream inFile(filename);
    if (inFile.is_open()) {
        Buch book;
        while (inFile >> book) {
            insertBook(book);
        }
        inFile.close();
        std::cout << "Data read from file successfully.\n";
    } else {
        std::cerr << "Unable to open file: " << filename << std::endl;
    }
}

I am expecting to be able to successfully add a book to the doubly linked list which i called Bibliothek (library).

Improve this question
7
  • OT: Don't do output in the input operator. How would that work if you want to read from a file instead? Commented Feb 7, 2024 at 19:53
  • 2
    When you have a big problem requiring many lines of code to express, try to make a smaller problem by isolating the problem in a small amount of code. Use minimal reproducible example for inspiration. Commented Feb 7, 2024 at 19:54
  • 2
    As for the crash, use a debugger to catch the crash and find out when and where in your code it happens. Commented Feb 7, 2024 at 19:55
  • The >> operator is not supposed to interact with the user. Do you really want prompts when you read from a file? It also normally reads the format that operator<< writes. Commented Feb 7, 2024 at 20:00
  • You make be able to use sanitizers found in many modern developments environments to assist your explorations with a debugger. A sanitizer will often tell you exactly what went wrong, where, and how. For example, here is the Address Sanitizer reporting an error encountered immediately on inputting 1 to add a new book. It says address 0x90, most likely a null pointer plus an offset, was accessed in Bibliothek::insertBook on line 335. tail->next = neuesBuecherfach. Likely tail is null the first time you add a book, so you cannot access next. Commented Feb 7, 2024 at 20:03

1 Answer 1

Reset to default
1

You should check and test each function before combining them in the whole program. Your code contains bugs.

For example, consider this code snippet from main

  switch (choice) {
      case 1: {
          Buch newBook;
          std::cin >> newBook;
          bookList.insertBook(newBook);
          std::cout << "Book added successfully.\n";
          break;
      }
      //...

This statement can insert a new object of the type Buch in the empty list bookList the pointers head and tail of which are null pointers.

Now let's investigate the function insertBook

void Bibliothek::insertBook(const Buch& buch) {
    Buecherfach* neuesBuecherfach = new Buecherfach(buch);
    if (head != nullptr) {
        head = tail = neuesBuecherfach;
        return;
    }

    Buecherfach* current = head;
    while (current) {
        if (current->data.getPrice() > buch.getPrice()) {
            if (current == head) {
                neuesBuecherfach->next = head;
                head->prev = neuesBuecherfach;
                head = neuesBuecherfach;
            } else {
                neuesBuecherfach->prev = current->prev;
                neuesBuecherfach->next = current;
                current->prev->next = neuesBuecherfach;
                current->prev = neuesBuecherfach;
            }
            return;
        }
        current = current->next;
    }

    tail->next = neuesBuecherfach;
    neuesBuecherfach->prev = tail;
    tail = neuesBuecherfach;
}

For starters this if statement

if (head != nullptr) {
    head = tail = neuesBuecherfach;
    return;
}

does not make sense. It seems you mean

if (head == nullptr) {
        ^^^^  
    head = tail = neuesBuecherfach;
    return;
}

If to return to your original code then this if statement will be skipped for an empty list. And the control will be passed to the following code

    Buecherfach* current = head;
    while (current) {
        if (current->data.getPrice() > buch.getPrice()) {
            if (current == head) {
                neuesBuecherfach->next = head;
                head->prev = neuesBuecherfach;
                head = neuesBuecherfach;
            } else {
                neuesBuecherfach->prev = current->prev;
                neuesBuecherfach->next = current;
                current->prev->next = neuesBuecherfach;
                current->prev = neuesBuecherfach;
            }
            return;
        }
        current = current->next;
    }

As the list is empty then the while loop will be also skipped. And after the loop you are trying to access memory using a null pointer

tail->next = neuesBuecherfach;

that results in undefined behavior.

I think your program has also other bugs.

Pay attention to that your design of the list is logically inconsistent. If the list is designed to store information in an order by some criteria then the functions push_front and push_back can break the order. That is they contradict the logic of the function insertBook.

And your list does not have a destructor and you should define the copy constructor and the copy assignment operator at least as deleted.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.