c++ vector title being a string

Question

I'm doing an university project on a library.

The class Book is used on data so I thought of creating a vector with pointers to the objects Book's. The problem is, I need to read an input file with the data and then create a Book to add to the vector<Book*> catalog.

Whenever I do so, the getline() with integers works fine, but as soon as I try to save a string data to the Book, such as Title, I get a total different string of characters instead of the desirable title, or worse, an error.

#ifndef LIBRARY_PROGRAM_BOOK_H
#define LIBRARY_PROGRAM_BOOK_H
#include <iostream>
#include <vector>
#include <string>
#include "Person.h"

class Book
{
public:
    //Book(std::string title, int id, );
    Book() {};
    ~Book(){};
    void setTitle(std::string title);
    std::string getTitle() const;
    void setBookId(int bookId);
    int getBookId() const;
    void setOwner(int owner);
    int getOwner() const;
    void setCategory(std::string category);
    std::string getCategory() const;
    void setValue(float value);
    float getValue() const;
    void setLoanFee(float loanFee);
    float getLoanFee() const;
    void setIsBorrowed(bool isBorrowed);
    bool getIsBorrowed() const;
    void setBeginOfLoan(int beginOfLoan);
    int getBeginOfLoan() const;
    void setMaximumLoanTime(int maximumLoanTime);
    int getMaximumLoanTime() const;
    void setEndOfLoan(int EndOfLoan);
    int getEndOfLoan() const;
    int getLoanTimeRemaining() const;
    void addRating(float rating);
    float getRating() const;
    void addComment(std::string comment);
    std::vector<std::string> getComments() const;
    //void addToWaitingList(Person* person);
    //bool removeFromWaitingList(Person* person);
    //void sortWaitingList();

private:
    std::string title; /**Book title*/
    int bookId; /**Book id*/
    int owner; /**Id from the book owner*/
    std::string category; /**Book category, i.e. Fiction, Drama, Police, ...*/
    float value; /**Book value*/
    float loanFee; /** Book loan fee*/
    bool isBorrowed; /** Boolean variable containing if the book is borrowed or not, true if yes, no otherwise*/
    int beginOfLoan; /**Will be set to 0 if book is not loaned in the moment */
    int maximumLoanTime; /**Max time that a book should be loaned, should be defined by the book owner*/
    int endOfLoan; /**Will be set to 0 if book is not loaned in the moment */
    std::vector<float> ratings; /**Vector containing all the ratings given by the readers*/
    std::vector<std::string> comments; /**Vector containing all the comments given by the readers*/
    //std::vector<Person*> waitingList; /**Vector containing who is waiting to read the book*/
    bool isLost; /** Boolean variable containing if the book is lost or not, true if yes, no otherwise*/
};

#endif //LIBRARY_PROGRAM_BOOK_H

This is the code where I read the file, create a Book, and add it to the catalog:

std::ifstream books_file ("../Books.txt");

// adding books to the catalog
while(!books_file.eof()) {
    Book book;
    std::getline(books_file, input);
    if(books_file.eof()) break;
    book.setTitle(input);
    std::getline(books_file, input);
    book.setBookId(std::stoi(input));
    std::getline(books_file, input);
    book.setOwner(std::stoi(input));
    catalog.push_back(&book);
}
books_file.close();

Does it have to do with the way I'm using pointers?

I tried doing vector<Book> instead of vector<Book*> and it works perfectly.

What am I not seeing right?

EDIT: error on Clion:

Process finished with exit code -1073741819 (0xC0000005)

uge = huge? `std::vector` already dynamically allocates its memory (and manages it for you), storing pointers in a vector will not save any space or allocations, it rather makes everything more complicated — 463035818_is_not_an_ai, Nov 02 '20 at 16:53
Also: https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons — AVH, Nov 02 '20 at 16:54
the books will be destroyed when they go out of scope, you store invalid pointers in the vector — 463035818_is_not_an_ai, Nov 02 '20 at 16:55
_Does it have to do with the way I'm using pointers?_ Yes it does, for the reason given in the preceding comment. — Paul Sanders, Nov 02 '20 at 16:56
`while(!books_file.eof())` is another bug. Details at [Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons). The code tries to hack around it with `if(books_file.eof()) break;`, bit this allows an EOF after that point to sneak through. Plus it's only looking for EOF and there are many more things in addition to EOF to trip over. — user4581301, Nov 02 '20 at 17:03
I highly recommend not using pointers. Use `std::vector` instead. Pointers will complicate the program. — Thomas Matthews, Nov 02 '20 at 17:12

score 3 · Accepted Answer · answered Nov 02 '20 at 17:05

3

while(!books_file.eof()) {
    Book book; <--------- stack allocated variable
    std::getline(books_file, input);
    if(books_file.eof()) break;
    book.setTitle(input);
    std::getline(books_file, input);
    book.setBookId(std::stoi(input));
    std::getline(books_file, input);
    book.setOwner(std::stoi(input));
    catalog.push_back(&book); <------- take address of stack allocation
} <----------- destroy stack allocation, making the address invalid

Solutions

make the vector contain books instead of pointers, if the vector is huge you will get hit by some copying/moving.

make a heap allocated copy of the book.

untested code

std::vector<std::unique_ptr<Book>> catalog.


auto book = std::make_unique<Book>(/* insert data for constructor of Book */);
catalog.emplace_back(std::move(book));

Using unique_ptr so the books gets destroyed when catalog does.

answered Nov 02 '20 at 17:05

Surt

15,501
3
23
39

This complexity can be eliminated by not using dynamic memory. Using `push_back()` will make a copy of the object and place into vector. – Thomas Matthews Nov 02 '20 at 17:13
I'll be better off by using vector then, thank you for the perfect explanation!! I have another vector, the difference is that I can add child classes like Member and Non Member, why when I create a NonMember* and Member*, add it to the vector, doesn't it happen the same? Is it because of polymorphism? – João Pedro Nov 02 '20 at 17:37
@ThomasMatthews The Book is a rather big structure and every reallocation will cause substantial copying, only a measuring can tell if its worth it. – Surt Nov 02 '20 at 17:53
@JoãoPedro you need the pointer when storing polymporphic objects because if you store the base class as a value, all of the derived class's extras [get sliced away](https://stackoverflow.com/questions/274626/what-is-object-slicing) to make the derived class fit into the base class. – user4581301 Nov 02 '20 at 17:59

score 0 · Answer 2 · answered Nov 02 '20 at 17:20

I think the root of your issue is that you create a Book right below the while line, take it's address when you put it in the vector, but then that Book immediately goes out of scope when you hit the } so it's destroyed and the pointer is pointing at essentially random memory. If you want to store a pointer to it you should allocate it on the heap via Book* book = new Book;. But as others have pointed out, you'd probably be better off storing the Book itself or a unique_ptr<Book> in your vector instead of raw pointers.

c++ vector title being a string

2 Answers2