Library management system in C++

Question

I am doing a beginner C++ project and it's simply a library management system where a student will hire, give back a book, book and hire records are stored in a binary file etc.

I'm approaching the project as though the project would be used by other developers and that I need to design the classes so they cannot be incorrectly used, the project can be scaled/extended at a later date easily and avoid gotchas/bugs down the track so I am being very strict (overkill almost) with my implementation.

Therefore, I have some features of the LibraryBook and LibraryManagementSystem classes that I want to implement but don't quite know if this is the best way of doing it.

Classes:

• LibraryManagementSystem (LMS)
• LibraryBook
• Student

LibraryBook features I want to implement:

• The class has the expected private properties of title, author, availability, id. I have made the LMS a friend class so that it and only it can change the books availability, maybe even author, title (for scalability/extendability). Would you use friend here or maybe another technique?
• A book object should only ever be created by the LMS, this makes sense I guess because a Student shouldn't be able to make a new book only the LMS. So I was thinking of making the LibraryBook class private but that means a Student won't even be able to hold a book (as a property in their class). I am thinking I should only ever give a handle of the book to the Student? Maybe this is the case for the application of the factory design pattern.
• A LibraryBook must always have its title, author, etc. defined, so I have made the constructor explicit. Should I make the default constructor private? That way a book can only ever be created with defined title, author, etc.

LMS features I want to implement:

• I want to store the LibraryBooks in a std::map <string, std::shared_ptr<LibraryBook>>. This is because a student will most likely ask/search for the book by title (title string = key), the LibraryBook dynamic memory will be automatically deleted and I can give the Student a weak_ptr to the LibraryBook which means they can't change anything on that object.

Knowing what I want to achieve above; can you provide advice on design patterns and etc. Have I made any errors in my implementation below?

class Student
{
 public:
 // I dont think that hire book should be a student method but a LMS method because the LMS issues a book, creates a record etc.
 weak_ptr<LibraryBook> curBook; // is this ok as public? or should only the LMS be able to set a students book? Maybe this property only should be friend with LMS?
 protected:
 private:
 const string firstName;
 const string lastName;
 const unsigned int ID;
};
class LibraryBook
{
 public:
 friend class LibraryManagementSystem;
 protected:
 private:
 explicit LibraryBook(string nTitle, string nAuthor) : title(nTitle), author(nAuthor) { } // will this mean noone not even LMS can create a book using the default // No need to implement destructor because implicitly created and theres no dyn memory in this class
 LibraryBook(const LibraryBook& lB); // dont implement to disallow copying a book
 LibraryBook& operator=(const LibraryBook& lB); // dont implement to disallow copying a book
 const string title;
 const string author;
 const unsigned int ID;
};
class LibraryManagementSystem
{
 public:
 static LibraryManagementSystem& Instance() 
 {
 static LibraryManagementSystem instance;
 return instance;
 }
 weak_ptr<LibraryBook> hireBook(string bookTitle, Student& student)
 {
 weak_ptr<LibraryBook> book; // initialised as null ptr I think?
 if (libraryBooks.find(bookTitle) == libraryBooks.end())
 // what should I return? An empty/null weak_ptr?
 book = libraryBooks[bookTitle]; // is this cast allowed/possible? Is it allowed to write/overwrite a weak ptr?
 student.curBook = book;
 // TODO: create LMS record of who has book, when due back, etc.
 return book;
 }
 protected:
 private:
 LibraryManagementSystem() {}
 ~LibraryManagementSystem() {}
 LibraryManagementSystem(const LibraryManagementSystem& lms); // dont implement to disallow copy constructor
 LibraryManagementSystem& operator=(const LibraryManagementSystem& lms); // dont implement assignment operator
 map<string, shared_ptr<LibraryBook>> libraryBooks;
};

• Quuxplusone: Before trying to give a detailed critique: (1) Why do you want to use dynamic memory management and pointers, instead of making LibraryBook a value type (e.g. storing the books in a std::set<LibraryBook> and handing out books by value)? (2) Do you care about thread-safety and multithreading? (3) What is the purpose of Student in this system? Can anyone besides Students check out a book? Can a single Student check out multiple books at once? Can books be returned to the library?
• sazr: @Quuxplusone 1. As I said I am looking for advice and to know if i have any errors. If it's better to use storing and giving values I am interested to know. The reason behind weak_ptr is to give student an object but not be able to edit it but i could pass a const reference instead. 2. Yes I'd like to implement thread safety. 3. Only students can hire, return books and only 1 at a time. Yes books can be returned.

Answer 1

Here's the general outline that I'd use for this problem.

Obviously in the real world we wouldn't bother with any of these classes at all, because we wouldn't be trying to make our Book class behave analogously to a real library book. (We'd probably just have one big SQL table with columns for "title" and "checked out by ___", and code to update the table in the obvious way.) But trying to make a skeuomorphic Book class seems like fun, so let's go for it.

#include <set>
#include <string>
#include <unordered_map>
#include <vector>
class Library {
 public:
 using Card = std::string;
 class Book {
 public:
 using Title = std::string;
 // Books are not copyable.
 Book(Book&&) = default;
 Book& operator=(Book&&) = default;
 Book(const Book&) = delete;
 Book& operator=(const Book&) = delete;
 ~Book() = default;
 Title getTitle() const { return title_; }
 private:
 // Allow the Library to create new Books.
 friend class Library;
 explicit Book(Title title): title_(title) {}
 Title title_;
 };
 // Just browsing...
 bool titleIsAvailable(const Book::Title&) const;
 // Show me a title and a valid card, and I'll give you a book.
 Book checkOutTitle(const Book::Title&, const Library::Card&);
 // Give me a book, and I'll put it back on the shelf.
 void returnBook(Book book);
 private:
 // The dopey hoops C++14 makes us jump through to get a working std::set::find...
 struct LessByTitle {
 using is_transparent = void;
 bool operator()(const Book& a, const Book& b) const { return a.getTitle() < b.getTitle(); }
 bool operator()(const Book& a, const std::string& b) const { return a.getTitle() < b; }
 bool operator()(const std::string& a, const Book& b) const { return a < b.getTitle(); }
 };
 std::set<Book, LessByTitle> shelf_;
 // We store a xerox copy of the student's Library::Card on file.
 std::unordered_map<Card, std::vector<Book::Title>> checkoutRecords_;
};

Also, to clear up one of your misconceptions from the comments: std::weak_ptr has nothing, zip, zilch, to do with "not being able to edit" the pointed-to object. What weak_ptr does is give you a weak reference that will automagically "expire" itself when the last strong reference is dropped — in your analogy, it would be like you're giving the Student a ticket redeemable for the real book, but with the possibility that the LibraryManagementSystem could burn the book in the meantime (in which case the ticket would become worthless). If the Student actually did redeem the ticket, by calling ticket.lock(), then the Student would receive a strong reference to the book, and could most certainly call any public member functions on the book that he wanted to.

Non-modifiability is basically the domain of const; strong and weak references are an advanced concept (unrelated to const) that you should probably be steering clear of for the time being. But heck, see these slides on smart pointers if you really want to learn about them right now.

If you use value semantics (i.e., passing Books around by value as in my outline above), you do still have to worry about modification: i.e., Students scribbling in Books and then returning them to the Library. This is solved basically by making the Book out of teflon: simply make sure all its public member functions are const. Right now the only public method implemented is getTitle, so we're all good there.

Does that make a certain amount of sense?

• Loki Astari: WE may indeed use SQL backing the data store. But the user of the LMS does not need know this (or how to use SQL) to access books. Hence a LMS is a good abstraction.
• Quuxplusone: My point was that in real life, the user of the computer system does not need a computer at all in order to access books; the books are physically sitting right there on the shelf next to him. What he needs the computer system for is to tabulate data about who's got which book. In real life we generally don't try to simulate books; we just tabulate data about books, using probably a simple GUI on top of some kind of database. There's no analogue to LibraryBook or Student or LibraryManagementSystem in the real-world computer system used by your local library.

Answer 2

book and hire records are stored in a binary file etc.

I would not do that.
It is a very inflexible format thus hard to update in the future. Also it makes debugging hard as it is not always human readable. Pick a format that is easy to parse both by humans and computers and is extensible.

JSON springs to mind.

The class has the expected private properties of title, author, availability, id. I have made the LMS a friend class so that it and only it can change the books availability, maybe even author, title (for scalability/extendability). Would you use friend here or maybe another technique?

That is a perfectly valid approach.

A book object should only ever be created by the LMS, this makes sense I guess because a Student shouldn't be able to make a new book only the LMS. So I was thinking of making the LibraryBook class private but that means a Student won't even be able to hold a book (as a property in their class).

If you want the student to access any methods on the book directly it needs to be public. But you can pass some identification to the Student object. It can then invoke methods on the book via the LMS and the book identification.

I am thinking I should only ever give a handle of the book to the Student? Maybe this is the case for the application of the factory design pattern.

Just give some unique identification of the book. This could be an ID or even just the title.

A LibraryBook must always have its title, author, etc. defined, so I have made the constructor explicit.

Making the construtor explicit in this case does nothing. explicit only applies to single argument constructors; when books are created by auto-converting another type into a Book by using the single argument constructor.

Should I make the default constructor private? You should not define a default constructor. Since you have already defined a constructor the compiler will not generate a default one for you.

I want to store the LibraryBooks in a std::map>.

You only need to use pointers (and thus `shared pointer) if you have different types of books. You may as well store the LibraryBook directly.

I would not bother to use a map. But rather a simple std::set just set the appropriate sort ability of the books.

This is because a student will most likely ask/search for the book by title (title string = key),

Just make the sort-ability of a book by on its title in the set declaration.

std::set<LibraryBook, LibraryBookSortOrder>

the LibraryBook dynamic memory will be automatically deleted and I can give the Student a weak_ptr to the LibraryBook which means they can't change anything on that object.

Weak pointer just detects if a shared pointer has been deleted. It does not prevent you from modifying the Book.

Changes

class LibraryBook
{
 friend class LibraryManagementSystem;
 private:
 LibraryBook(std::string const& nTitle, std::string const& nAuthor)
 : title(nTitle)
 , author(nAuthor)
 { }
 LibraryBook(LibraryBook const&) = delete;
 LibraryBook& operator=(LibraryBook const& lB) = delete;
 std::string title;
 std::string author;
 std::size_t count; // copies of the book owned
 std::vector<StudentID> onLoanTo; // who has copies.
 friend operator<<(std::ostream& str, LibraryBook const& book)
 {
 return str << title << " : " << author << "\n";
 }
};

LMS

class LibraryManagementSystem
{
 public:
 typedef std::set<LibraryBook, LibraryBookSortOrder> Storage;
 typedef Storage::const_iterator BookRef;
 static LibraryManagementSystem& Instance() 
 {
 static LibraryManagementSystem instance;
 return instance;
 }
 BookRef hireBook(std::string const& bookTitle, StudentId student)
 {
 auto find = libraryBooks.find(bookTitle);
 if (find == libraryBooks.end()) {
 return find;
 }
 if (find->count == find->onLoanTo.size()) {
 return libraryBooks.end();
 }
 find->onLoanTo.push_back(student);
 return find;
 }
 private:
 LibraryManagementSystem()
 {
 // Load books from backing store
 }
 LibraryManagementSystem(const LibraryManagementSystem&) = delete;
 LibraryManagementSystem& operator=(const LibraryManagementSystem&) = delete;
 Storage libraryBooks;
};

• Edward: Plain string in what is likely to be in a header file is suspect. You might want to use std::string in your rewrite and point out the hazards of a possible using namespace std; in a header.
• Loki Astari: @Edward: fixed.
• Quuxplusone: "Making the constructor explicit in this case does nothing. explicit only applies to single argument constructors" — technically, no; explicit disables implicit conversions in all cases, including e.g. LibraryBook foo() { return {"foo","bar"}; }. It's a subtle point, but still, I'd generally advise making all (non-special) constructors explicit, because otherwise you have to worry about whether to add explicit on constructors that take (int, int=0)... ones that take (Args&&...)... ones that take (int,...)... et cetera. It's just foolproof'er to add explicit on everything.