Library Management System with OOP [closed]

Question

I am creating a library management system. Presently am done creating the Book and BookItem class. Can you point any thing you feel isn't necessary, inefficient or a general bad practice so I could learn from it and grow in my programming career

Book.hh

#ifndef BOOK_HH
#define BOOK_HH
/*****************************************************************
 * Name: Book.hh
 * Author: Samuel Oseh
 * Purpose: Book class method-function prototype
 * ***************************************************************/

#include <string>
class Book {
    public:
        /* method function */
        Book()  = default;
        ~Book(){}
        virtual std::string getStatus() const = 0;
        virtual std::string getType() const = 0;
};

#endif

BookItem.hh

#ifndef BOOKITEM_HH
#define BOOKITEM_HH
/*****************************************************************
 * Name: BookItem.hh
 * Author: Samuel Oseh
 * Purpose: BookItem class method-function prototype
 * ***************************************************************/
#include "Book.hh"
#include <string>
#include <ctime>

class BookItem : Book{
    private:
        /* data */
        std::string name;
        std::string author;
        std::string pubDate;
        std::string isbn;
        std::string status;
        std::string type;
        bool onlineCopy{false};
        /* utility function */
        bool validatePubDate( std::string date ) const;
        
    public:
        BookItem() = default;
        BookItem( const std::string &name, const std::string author, const std::string pubDate, \
                const std::string isbn, const std::string status, const std::string type, bool onlineCopy = false ); 
        BookItem( const BookItem &bookItem ) { *this = std::move(bookItem); }
        BookItem& operator=( const BookItem &bookItem );
        void setStatus( std::string status );
        void setType( std::string status );
        std::string getStatus() const { return status; };
        std::string getType() const { return type; }
        void setOnlineCopy() { onlineCopy = true; }
        bool hasOnlineCopy() { return onlineCopy; }
        std::string giveName() const { return name; }
        std::string giveAuthor() const { return author; }
        std::string givePubDate() const { return pubDate; }
        std::string giveIsbn() const { return isbn; }
        
        static const std::string *const statusPtr;
        static const std::string *const typePtr;
        
        ~BookItem(){}

};

#endif

BookItem.cc

/*****************************************************************
 * Name: BookItem.cc
 * Author: Samuel Oseh
 * Purpose: BookItem class method-function definitions
 * ***************************************************************/
#include <iostream>
#include <stdexcept>
#include "BookItem.hh"

std::string statusCopy[5] = { "RESERVED", "AVAILABLE", "UNAVAILABLE", "REFERENCE", "LOANED" };
std::string typeCopy[] = {"ART", "BASIC MEDICAL SCIENCES", "LAW", "PROGRAMMING", "COMPUTER SCIENCE", "NURSING", "PHARMARCY", "MAGAZINE",\
                            "ARTICLE", "JOURNAL", "BANKING", "NEWSLETTER"};
const std::string *const BookItem::typePtr = typeCopy;
const std::string *const BookItem::statusPtr = statusCopy;

BookItem:: BookItem( const std::string &nme, const std::string author, const std::string pubDate, \
                const std::string isbn, const std::string status, const std::string type, bool onlineCopy ) : Book() {
                    this->name = name;
                    this->author = author;
                    if ( validatePubDate( pubDate ) )
                        this->pubDate = pubDate;
                    else 
                        throw std::invalid_argument("Invalid publication date");
                    this->isbn = isbn;
                    setStatus( status );
                    setType( type );
                    this->onlineCopy = onlineCopy;
}


BookItem &BookItem::operator=( const BookItem &bookItem ) {
    name = bookItem.name;
    author = bookItem.author;
    pubDate = bookItem.pubDate;
    isbn = bookItem.pubDate;
    status = bookItem.status;
    type = bookItem.type;
    onlineCopy = bookItem.onlineCopy;
    return *this;
}

bool BookItem::validatePubDate( std::string date ) const { 
    struct tm tm;
    if ( strptime( date.c_str(), "%d/%m/%y", &tm ) ) 
        return true;
    return false;
}

void BookItem::setStatus( std::string status ) {
    for ( const std::string *p = begin(statusCopy); p != end(statusCopy); ++p ) {
        if ( *p == status ) {
            this->status = status;
            return;
        }
    }
    throw std::invalid_argument("Invalid Status");
}

void BookItem::setType( std::string type ) {
    for ( const std::string *p = begin(typeCopy); p != end(typeCopy); ++p ) {
        if ( *p == type ) {
            this->type = type;
            return;
        }
    }
    throw std::invalid_argument("Invalid Type");
}
```

Answer 1

1. First issue is that I don't see why you make it via OOP. Why Book is an interface? It has almost zero functionality. Use interface for objects with complex functionality when you have several possible implementations. While the Book class can only serve for downcasting which in itself is best avoided unless necessary. Also it has a bug that the destructor isn't virtual - it needs to be virtual when you work with abstract classes / polymorphism.

2. Throwing in a constructor is a bad idea in general and especially in OOP cases. If classes weren't fully constructed then a destructor might mess up and corrupt the code leading to UBs - so to throw in a constructor programmer needs to make sure that such issues do not occur and there are some complex cases where it is non-trivial. You should avoid it by storing date not as a string but as a dedicated class/struct. And let user call the conversion from string when needed. If the conversion fails than throw will happen outside of the constructor.

3. BookItem& operator=( const BookItem &bookItem ); why implement it? Just default it or don't write at all. And declaring destructor ~BookItem(){} is pointless. Also you are missing move assignment and move constructor.

4. You should use enum class for BookType and BookStatus. No idea why you use a string - it is just a waste and confusion for users. Just add general string to/from enum conversions.

5. std::string giveName() const ... and similar have weird names. WTF does giveName means? I don't understand it. It should be getName and same for other giveXXX replace it with getXXX. Furthermore, books don't have names: they have titles. So replace "name" with "title".

6. The functions std::string giveAuthor() and the like return std::string which potentially make an unnecessary allocation and allocations are slow and cause memory fragmentation in the long run. You can avoid it by either returning const reference const std::string& or std::string_view.

7. void setOnlineCopy() ... also doesn't follow naming conventions. People use set to supply interface to set value to given property like void setX(int x){mX = x;} but here you just set the boolean to true. Declare the function like void setOnlineCopyStatus(bool bHasOnlineCopy).