Object Oriented Library Management System

Question

Library management system is a object oriented program that takes care of the basic housekeeping of a library. This is the third part of a series. The first iteration of the project is found here and the second iteration is also found here

The major actors are the Librarian, Member, and the System.

Major Concern

I made member and Librarian friends of the system class and also composed the system class with Librarian and Member. I felt "System is composed of a Librarian and members" sounds better than "System is a friend of a Librarian and members". Does this follow the common design patterns?

In my previous post, I wasn't cleared well enough on why a std::vector should be preferable than std::list in this situation due to the fact that the system would perform insertion frequently. Would std::vector be more scalable and versatile whilst taking speed and efficiency into consideration?

Any other observation on potential pitfalls, traps and common bad practice can be pointed out.

Date.hh

#ifndef DATE_HH
#define DATE_HH
class Date {
 friend std::ostream &operator<<( std::ostream &, const Date & );
 private:
 /* data-members */
 unsigned month = 1;
 unsigned day = 1;
 unsigned year = 1970;
 /* utility functions */
 bool validateDate( unsigned m, unsigned d = 1, unsigned y = 1970 );
 bool checkDay( unsigned m, unsigned d, unsigned y ) const;
 bool isLeapYear( unsigned y ) const { return ( y % 400 == 0 ) || ( y % 4 == 0 && y % 100 != 0 ); }
 public:
 static constexpr unsigned int monthsPerYear = 12;
 /* ctors */
 Date() = default;
 Date( unsigned m, unsigned d, unsigned y );
 Date( unsigned m );
 Date( unsigned m, unsigned d );
 
 /* copy operations */
 Date( const Date &d ) = default;
 Date &operator=( const Date &d ) = default;
 /* equality test operations */
 bool operator==( const Date &d ) const;
 bool operator!=( const Date &d ) const { return !( *this == d ); }
 /* method-functions */
 void setDate( unsigned m = 1, unsigned d = 1, unsigned y = 1970 );
 unsigned getMonth() const;
 unsigned getDay() const;
 unsigned getYear() const; 
 void nextDay();
 const std::string toString() const;
 // dtor
 ~Date(){};
};
#endif

Date.cc

#include <iostream>
#include <string>
#include <stdexcept>
#include <array>
#include "../headers/Date.hh"
Date::Date( unsigned m, unsigned d, unsigned y ) { 
 if ( validateDate(m, d, y ) ) {
 month = m; day = d; year = y;
 }
}
Date::Date( unsigned m ) {
 if( validateDate( m ) )
 month = m;
}
Date::Date( unsigned m, unsigned d ) {
 if ( validateDate( m, d ) ) {
 month = m; day = d;
 }
}
void Date::setDate( unsigned m, unsigned d, unsigned y ) {
 if ( validateDate( m, d, y ) ) {
 month = m; day = d; year = y;
 }
}
void Date::nextDay() {
 day += 1;
 try {
 checkDay( month, day, year );
 } catch ( std::invalid_argument &e ) {
 month += 1;
 day = 1;
 }
 if ( month % 12 == 0 ) {
 year += 1;
 month = 1;
 }
}
bool Date::operator==( const Date &d ) const {
 if( month != d.month ) return false;
 if ( day != d.day ) return false;
 if ( year != d.year ) return false;
 return true;
}
std::ostream &operator<<( std::ostream &os, const Date &d ) {
 os << d.month << "/" << d.day << "/" << d.year;
 return os;
} 
// utility function
bool Date::validateDate( unsigned m, unsigned d, unsigned y ) {
 // validate month
 if ( m < 1 || m >= 13 )
 throw std::invalid_argument( "Month must be between 1-12" );
 // validate day
 if ( checkDay( m, d, y ) == false )
 throw std::invalid_argument( "Invalid day for current month and year" );
 
 // validate year
 if ( y < 1970 )
 throw std::invalid_argument( "year must be greater than 1969" );
 
 return true;
}
 const std::string Date::toString() const {
 return std::to_string(month) + "/" + std::to_string(day) + "/" + std::to_string(year);
 }
bool Date::checkDay( unsigned testMonth, unsigned testDay, unsigned testYear ) const {
 static const std::array < unsigned, monthsPerYear + 1 > daysPerMonth = { 0,31,28,31,30,31,30,31,31,30,32,30,31};
 if ( testDay > 0 && testDay <= daysPerMonth[ testMonth ] )
 return true;
 
 if ( testMonth == 2 && testDay == 29 && isLeapYear( testYear ) ) 
 return true;
 return false;
}

BookItem.hh

#ifndef BOOKITEM_HH
#define BOOKITEM_HH
#include <iostream>
#include <string>
#include <string_view>
#include "Date.hh"
enum class BookStatus : unsigned { RESERVED, AVAILABLE, UNAVAILABLE, REFERENCE, LOANED, NONE };
enum class BookType : unsigned { HARDCOVER, MAGAZINE, NEWSLETTER, AUDIO, JOURNAL, SOFTCOPY };
class BookItem {
 friend std::ostream &operator<<( std::ostream &, const BookItem & );
 private:
 /* data-members */
 std::string title;
 std::string author;
 std::string category;
 Date pubDate;
 std::string isbn;
 BookStatus status;
 BookType type;
 /* user connected to this book */
 std::string bookcurrentUser;
 public:
 /* ctors */
 BookItem() = default;
 BookItem( const std::string &title, const std::string &author, const std::string &cat, const Date &pubDate, \
 const std::string &isbn, const BookType type, const BookStatus status = BookStatus::AVAILABLE ); 
 bool operator==( const BookItem &bookItem ) const;
 bool operator!=( const BookItem &bookItem ) const { return !( *this == bookItem); };
 /* method-functions */
 void setStatus( BookStatus s ) { status = s; };
 void setType( BookType t ) { type = t;};
 void setCategory( const std::string &c ) { category = c; }
 void setBookCurrentUser( std::string userName ) { bookcurrentUser = userName; }
 std::string_view getBookCurrentUser() const { return bookcurrentUser; }
 std::string_view getStatus() const;
 std::string_view getType() const;
 std::string_view getTitle() const { return title; }
 std::string_view getAuthor() const { return author; }
 std::string_view getCategory() const { return category; };
 std::string_view getIsbn() const { return isbn; }
 Date &getPubDate() { return pubDate; }
 void printPubDate() const { std::cout << pubDate; } 
 const BookStatus getStatusByEnum() const { return status; }
 const BookType getTypeByEnum() const { return type; }
 // dtor
 ~BookItem() = default;
};
#endif

BookItem.cc

#include <iostream>
#include "../headers/BookItem.hh"
BookItem::BookItem( const std::string &t, const std::string &a, const std::string &c, const Date &d, \
 const std::string &i, const BookType ty, const BookStatus s ) {
 title = t, author = a, category = c, pubDate = d, isbn = i;
 setStatus( s );
 setType( ty );
}
bool BookItem::operator==( const BookItem &bookItem ) const {
 if ( title != bookItem.title ) return false;
 if ( author != bookItem.author ) return false;
 if ( category != bookItem.category ) return false;
 if ( pubDate != bookItem.pubDate ) return false;
 if ( isbn != bookItem.isbn ) return false;
 if ( status != bookItem.status ) return false;
 if ( type != bookItem.type ) return false;
 return true;
}
std::string_view BookItem::getStatus() const { 
 switch( status ) {
 case BookStatus::AVAILABLE:
 return "AVAILABLE";
 case BookStatus::REFERENCE:
 return "REFERENCE";
 case BookStatus::UNAVAILABLE:
 return "UNAVAILABLE";
 case BookStatus::LOANED:
 return "LOANED";
 case BookStatus::RESERVED:
 return "RESERVED";
 default:
 return "NONE";
 }
} 
std::string_view BookItem::getType() const {
 switch( type ) {
 case BookType::AUDIO:
 return "AUDIO";
 case BookType::HARDCOVER:
 return "HARDCOVER";
 case BookType::JOURNAL:
 return "JOURNAL";
 case BookType::MAGAZINE:
 return "MAGAZINE";
 case BookType::NEWSLETTER:
 return "NEWSLETTER";
 case BookType::SOFTCOPY:
 return "SOFTCOPY";
 default:
 return "NONE";
 }
}
std::ostream &operator<<( std::ostream &os, const BookItem &b ) {
 os << "\nName of book: " << b.getTitle();
 os << "\nAuthor of book: " << b.getAuthor();
 os << "\nBook category: " << b.getCategory();
 os << "\nPublication date: " << b.pubDate;
 os << "\nISBN number: " << b.getIsbn();
 os << "\nStatus of book: " << b.getStatus();
 os << "\nType of book: " << b.getType();
 return os;
}

Librarian.hh

#ifndef LIBRARIAN_HH
#define LIBRARIAN_HH
#include <iostream>
#include <string>
#include "BookItem.hh"
class System;
class Librarian {
 public:
 /* data-members */
 std::string name;
 Date dateOfHire;
 /* ctors */
 Librarian() = default;
 Librarian( const std::string &name, const Date &dateOfHire );
 // basic method-function
 void printDateOfHire() const { std::cout << dateOfHire; }
 /* core functionalities */
 void addBook( System &sys, const BookItem &isbn );
 void removeBook( System &sys, const std::string &isbn );
 void auditLibrary( const System &sys ) const;
 // dtor
 ~Librarian(){}
};
#endif

Librarian.cc

#include <iostream>
#include "../headers/System.hh"
#include "../headers/Librarian.hh"
Librarian::Librarian( const std::string &n, const Date &d ) {
 name = n;
 dateOfHire = d;
}
void Librarian::addBook(System &sys, const BookItem &book ) { 
 if ( sys.books.empty() ) {
 sys.books.push_front( book );
 return;
 }
 for ( auto bptr = sys.books.cbegin(); bptr != sys.books.cend(); ++bptr ) {
 if( book.getTitle() <= bptr->getTitle() ) {
 sys.books.insert(bptr, book);
 return;
 }
 }
 sys.books.push_back( book );
}
void Librarian::removeBook( System &sys, const std::string &isbn ) {
 BookItem book = sys.getBook( isbn );
 for ( auto bptr = sys.books.cbegin(); bptr != sys.books.cend(); ++bptr ) {
 if ( book.getIsbn() == bptr->getIsbn() ) {
 sys.books.remove(book);
 std::cout << "Deleted { " << book.getAuthor() << " : " << book.getTitle() << " } \n";
 return;
 }
 }
 throw std::invalid_argument("Book not found");
}
void Librarian::auditLibrary( const System &sys ) const {
 std::cout << "\nName of Library: " << sys.libraryName << ", Date created " << sys.dateCreated;
 std::cout << "\nLibrarian: " << name << ", Date of hire: " << dateOfHire;
 std::cout << "\n\nBooks: ";
 for ( auto bptr = sys.books.cbegin(); bptr != sys.books.cend(); ++bptr ) {
 std::cout << *bptr << "\n";
 std::cout << "This book is linked to: " 
 << ( ( bptr->getBookCurrentUser() == "" ) ? "None" : bptr->getBookCurrentUser() ) << "\n"; 
 }
 std::cout << "\n\nMembers: ";
 for ( auto mPtr = sys.members.cbegin(); mPtr != sys.members.cend(); ++mPtr ) {
 std::cout << *mPtr << "\n";
 }
}

Member.hh

#ifndef MEMBER_HH
#define MEMBER_HH
#include <string>
#include <vector>
#include "Date.hh"
#include "BookItem.hh"
class System;
class Member {
 friend std::ostream& operator<<( std::ostream&os, const Member &m );
 private:
 /* data-member */
 std::string libraryNumber;
 Date dateRegisted;
 std::vector<BookItem> checkedOutBooks;
 public:
 /* data-member */
 std::string name;
 char sex;
 /* ctors */
 Member() = default;
 Member( const std::string &n, const char s, Date d ) : dateRegisted( d ), name( n ), sex( s ) {}
 
 /* method-functions */
 std::string getLibraryNumber() const { return libraryNumber; }
 void setLibraryNumber( const std::string &lNum ) { libraryNumber = lNum; };
 void checkOut( System &sys, const std::string &isbn );
 void returnBook( System &sys, const std::string &isbn );
 bool operator==( const Member &m );
 bool operator!=( const Member &m ) { return !( *this == m ); }
 // dtor
 ~Member() = default;
};
#endif

System.cc

#ifndef SYSTEM_HH
#define SYSTEM_HH
#include <string>
#include <list>
#include <vector>
#include "Date.hh"
#include "BookItem.hh"
#include "Librarian.hh"
#include "Member.hh"
class System {
 friend class Librarian;
 friend class Member;
 private:
 /* data-members */
 std::list<BookItem> books{};
 std::vector<Member> members{};
 Librarian librarian;
 Member member;
 public:
 /* ctors */
 System() = default;
 System( const std::string &name, Date &date ) : libraryName( name ), dateCreated( date ) {};
 /* method-functions */
 const std::string generateLibraryNumber() const;
 void addMember( Member &m ) { members.push_back( m ); };
 void deleteMember( Member &m );
 void displayMembers();
 BookItem getBook( const std::string &isbn ) const;
 void viewBook( const std::string isbn ) const;
 void placeOnReserve( const std::string, const std::string &isbn );
 void displayAllBooks() const;
 /* data-members */
 std::string libraryName;
 Date dateCreated;
 
 
 /* search functionalities */
 std::list<BookItem> queryByTitle( const std::string &t ) const;
 std::list<BookItem> queryByAuthor( const std::string &a ) const;
 std::list<BookItem> queryByPubDate( const Date &d );
 std::list<BookItem> queryByStatus( const BookStatus &s ) const;
 std::list<BookItem> queryByType( const BookType &ty ) const;
 // dtor
 ~System() = default;
};
#endif

System.cc

#include <iostream>
#include <set>
#include "../headers/System.hh"
std::list<BookItem> System::queryByTitle( const std::string &t ) const {
 std::list<BookItem> queryList;
 for ( auto bPtr = books.cbegin(); bPtr != books.cend(); ++bPtr ) {
 if ( bPtr->getTitle().find(t) != std::string::npos )
 queryList.push_back( *bPtr );
 }
 return queryList;
}
std::list<BookItem> System::queryByAuthor( const std::string &a ) const {
 std::list<BookItem> queryList;
 for ( auto bPtr = books.cbegin(); bPtr != books.cend(); ++bPtr ) {
 if ( bPtr->getAuthor().find(a) != std::string::npos )
 queryList.push_back( *bPtr );
 }
 return queryList;
}
std::list<BookItem> System::queryByPubDate( const Date &d ) {
 std::list<BookItem> queryList;
 for ( auto bPtr = books.begin(); bPtr != books.cend(); ++bPtr ) {
 if ( bPtr->getPubDate().toString().find(d.toString()) != std::string::npos )
 queryList.push_back( *bPtr );
 }
 return queryList;
}
std::list<BookItem> System::queryByStatus( const BookStatus &s ) const {
 std::list<BookItem> queryList;
 for ( auto bPtr = books.begin(); bPtr != books.cend(); ++bPtr ) {
 if ( bPtr->getStatusByEnum() == s )
 queryList.push_back( *bPtr );
 }
 return queryList;
}
std::list<BookItem> System::queryByType( const BookType &ty ) const {
 std::list<BookItem> queryList;
 for ( auto bPtr = books.begin(); bPtr != books.cend(); ++bPtr ) {
 if ( bPtr->getTypeByEnum() == ty )
 queryList.push_back( *bPtr );
 }
 return queryList;
}
void System::placeOnReserve( const std::string name, const std::string &isbn ) {
 for ( auto bPtr = books.begin(); bPtr != books.end(); ++bPtr ) {
 if ( bPtr->getIsbn() == isbn ) {
 bPtr->setStatus( BookStatus::RESERVED );
 bPtr->setBookCurrentUser( name );
 }
 }
}
BookItem System::getBook( const std::string &isbn ) const {
 for ( auto bPtr = books.cbegin(); bPtr != books.cend(); ++bPtr ) {
 if ( bPtr->getIsbn() == isbn )
 return *bPtr;
 }
 throw std::invalid_argument("Book is not available at the library");
}
void System::viewBook( const std::string isbn ) const {
 std::cout << getBook( isbn ); 
}
const std::string System::generateLibraryNumber() const {
 static std::string Codes[10]{"XGS", "QWT", "OPI", "NMK", "DXF", "PXG", "OPI", "QPU", "IKL", "XYX" };
 static std::set< unsigned, std::greater<unsigned> > idSet;
 unsigned id;
 bool unique = false;
 unsigned index = 0 + rand() % 9;
 std::string code = Codes[ index ];
 while ( unique == false ) {
 id = 10000000 + rand() % 9999999;
 auto ret = idSet.emplace(id);
 if ( !ret.second ) {
 std::cout << "unique failed";
 unique = false;
 continue;
 }
 else 
 unique = true;
 }
 return code + std::to_string( id );
}
void System::deleteMember( Member &m ) {
 for ( auto mPtr = members.begin(); mPtr != members.end(); ++mPtr ) {
 if ( *mPtr == m ) {
 members.erase( mPtr );
 std::cout << "Deleted member: { Name: " << m.name << ", Library Number: " << m.getLibraryNumber() << 
 " }\n";
 return;
 }
 }
 throw std::invalid_argument("No such member found");
}
void System::displayMembers() {
 std::cout << "Members of Library: ( count : " << members.size() << " ) " << "\n";
 for ( auto mPtr = members.cbegin(); mPtr != members.cend(); ++mPtr ) {
 std::cout << *mPtr;
 }
}
void System::displayAllBooks() const {
 for ( auto bPtr = books.begin(); bPtr != books.end(); ++bPtr ) {
 std::cout << *bPtr << "\n\n";
 }
}
```

Answer 1

Date:

• Date.hh is missing some includes (<iostream>, <string>).

• Don't supply a default constructor. It doesn't make sense to have a default date.

• Don't supply one- and two-argument constructors. Specifying a month and date in 1970 is unlikely to be very common.

• We should support years before 1970. There were books back then!

• year should be a signed number (even if that capability is unlikely to be used).

• day and month can be smaller types (e.g. std::uint8_t).

• setDate() is unnecessary, since we have the constructor and assignment.

• One would expect a function called nextDay() to return a copy, rather than modifying the Date instance (c.f. standard library iterators next and advance).

• If the destructor does nothing, we can omit it.

• validateDate can never return false, so it should have a void return value (and perhaps be called throwIfInvalid or something similar).

• Member functions that don't need access to a class instance's member variables (validateDate etc.) can be made static.

• I'd suggest printing dates as "yyyy-mm-dd" (or printing the month by name). Putting the day in the middle is highly illogical.

• If you have C++20, use std::chrono::year_month_day instead!

---

BookItem:

• We should use the constructor initializer list to initialize member variables.

• Again, we don't want a default constructor.

• We don't need to specify a destructor.

• Note that libraries often have several copies of the same book. When a book has an ISBN (mainstream publications after 1970), we don't need to duplicate the book data (title, author, etc.) for every copy of the book in the library. Perhaps we should move the book data into a separate class, and have a std::variant<ISBN, BookData> in BookItem? (But maybe that's going too far for this implementation).

• We should add a unique identifier for each item the library holds.

---

Librarian:

• addBook and removeBook should not be part of this class. They modify the System class internals, and should be part of the System class. auditLibrary should be moved there too.

• The default constructor shouldn't exist. The destructor doesn't need to exist.

• I don't think this class needs to exist at all for the current library functionality.

---

Member:

• Default constructor bad. Destructor unnecessary.

• We don't really want to store BookItems here by value. We just need to store an ID for each item they have checked out.

• checkOut and returnBook shouldn't be here, they should be part of System.

---

System:

• Shouldn't have friends.

• Should perhaps be called Library.

• Don't worry about speed unless it actually becomes a problem. There's probably no point in even storing books by title (we're as likely to want to search by author or category or publication date or...).

• (Note that the title search doesn't take into account that the list is sorted by title!)

• std::list has few valid uses. It only becomes faster than std::vector for inserting and removing many (hundreds of thousands) of items from the middle of the list. std::vector would be fine here.

• Use range-based for loops where appropriate: for (auto const& i: books) { ... }.

• Note that the <algorithm> header supplies various functions to find and copy things.

Answer 2

I've read answer by @user673679 and just want address a few issues.

I strongly disagree with disabling default constructors in classes like Member/Date/BookItem. If class has no default constructor then using it with std::vector, std::map and other template containers becomes very awkward in general. So it is a bad advise.

Instead you should make default constructed classes such as these be clearly uninitialized and add functions/methods that test it.

Another note: I want to expand upon std::vector vs std::list. std::list is a very slow container and unsuitable for most purposes. It requires a new allocation for each element and to find a middle element one needs to traverse half the list. So using std::list is almost a sacrilege. There a few rare cases where list could be beneficial but definitely not in this code.

Use std::vector instead. You'll need to eventually figure out how to properly manage memory and lookup for searching but use std::vector for storing the base classes. For instance, erasing a single element isn't worth rearranging the whole vector. Instead, just keep count of number of empty locations and if the number surpasses half the total size then rearrange it.

You still lack move constructor and move assignment for BookItem. Having it will improve performance of classes like std::vector that hold the book items.

• theProgrammer: I have no idea about move constructor, Am currently learning with c++ primer and I haven't yet gotten to the chapter were move constructors are discussed, I just wrote the program with my current knowledge.
• theProgrammer: Please elaborate on "Instead you should make default constructed classes such as these be clearly uninitialized and add functions/methods that test it."
• G. Sliepen: There sometimes are legitimate reasons for disabling default constructors, although here I don't think there is an issue to have default constructors indeed.
• ALX23z: @G.Sliepen there are legitimate reasons to disable various constructors including default ones. But the ones listed weren't such.
• ALX23z: @theProgrammer e.g. you can by default instantiate the data with invalid day/month or simply have a boolean that states whether the class was instantiated. You may or may not differentiate the cases "invalid date" and "non initialized date" - it is up to your choice of design. Though, I believe the two cases are quite different and should be treated differently. Alternatively, you can utilize std::optional.