patterncppMinor
Inserting and displaying books
Viewed 0 times
insertingandbooksdisplaying
Problem
I'm trying to write a program where you can insert and display some books (without using a database).
For doing this, I use three classes:
```
#include
#include
#include
using namespace std;
class Book {
public:
string author, title;
bool rented;
Book(string &author, string &title, bool rented) {
this -> author = author;
this -> title = title;
this -> rented = rented;
};
void display(void);
};
class TehnicBook:public Book {
int amount, RlYear;
string language;
TehnicBook head, next;
public:
TehnicBook(string &author, string &title, bool rented, int amount, string &language, int RlYear):Book(author, title, rented) {
head = NULL;
this -> language = language;
this -> amount = amount;
this -> RlYear = RlYear;
};
~TehnicBook(void) {
delete head;
};
void display(void);
void add(void);
void dellete(string&);
};
class Literature:public Book {
string bookType;
Literature head, next;
public:
Literature(string &author, string &title, bool rented, string &bookType):Book(author, title, rented) {
head = NULL;
this -> bookType = bookType;
};
~Literature(void) {
delete head;
};
void display(void);
void add(void);
};
void TehnicBook::add(void) {
string author, title, language;
int year, amount;
bool rented;
cout > author;
cout > title;
cout > rented;
cout > amount;
cout > language;
cout > year;
TehnicBook *p = new TehnicBook(author, title, rented, amount, language, year);
p -> next = head;
head = p;
}
void TehnicBook::display(void) {
TehnicBook *p = head;
while(p) {
cout author title rented) ? "" : "not ") amount language RlYear next;
}
}
void Literature::add(void) {
string author, title, bookType;
For doing this, I use three classes:
Book- is the base class.
TehnicBookandLiterature- that will inherit some properties fromBook.
```
#include
#include
#include
using namespace std;
class Book {
public:
string author, title;
bool rented;
Book(string &author, string &title, bool rented) {
this -> author = author;
this -> title = title;
this -> rented = rented;
};
void display(void);
};
class TehnicBook:public Book {
int amount, RlYear;
string language;
TehnicBook head, next;
public:
TehnicBook(string &author, string &title, bool rented, int amount, string &language, int RlYear):Book(author, title, rented) {
head = NULL;
this -> language = language;
this -> amount = amount;
this -> RlYear = RlYear;
};
~TehnicBook(void) {
delete head;
};
void display(void);
void add(void);
void dellete(string&);
};
class Literature:public Book {
string bookType;
Literature head, next;
public:
Literature(string &author, string &title, bool rented, string &bookType):Book(author, title, rented) {
head = NULL;
this -> bookType = bookType;
};
~Literature(void) {
delete head;
};
void display(void);
void add(void);
};
void TehnicBook::add(void) {
string author, title, language;
int year, amount;
bool rented;
cout > author;
cout > title;
cout > rented;
cout > amount;
cout > language;
cout > year;
TehnicBook *p = new TehnicBook(author, title, rented, amount, language, year);
p -> next = head;
head = p;
}
void TehnicBook::display(void) {
TehnicBook *p = head;
while(p) {
cout author title rented) ? "" : "not ") amount language RlYear next;
}
}
void Literature::add(void) {
string author, title, bookType;
Solution
-
Major bug:
If you have C++11 I recommend that every class either has a
-
Major design issue: You are combining
-
Similarly
-
Similarly
Now you can do
Major bug:
Book::~Book is not virtual. This means you have a memory leak in the following case:TehnicBook *tb = new TehnicBook(...);
tb->add(...);
Book *b = tb;
delete tb; //head is leakedIf you have C++11 I recommend that every class either has a
virtual destructor or is declared final such as class Book final{...}.-
Major design issue: You are combining
TehnicBook with a linked list. A class should have only one responsibility: Either manage a book or manage memory, not both. At least use std::list or better std::vector. Better remove add from TehnicBook and Literature and put a vector into main to seperate storage and functionality.-
Similarly
add should not use cin and cout. You are mixing user interface and class functionality. It should simply take a TehnicBook *. You can make a free standing function that conveniently does this, but it should not be part of TehnicBook.-
Similarly
display should not use cout. Either make it return a string which you then give to cout or teach cout how to print books such as this:ostream &operator author title rented) ? "" : "not ") amount language RlYear << endl;
os << endl;
return os;
}Now you can do
TehnicBook book(...); cout
-
I see void dellete(string&); inside TehnicBook, but no implementation. I guess you meant to add the functionality to remove books from the list. Remove this function, it does not belong to a TehnicBook.
-
Is TehnicBook supposed to be TechnicBook or TechnicalBook?
-
This may be a bit over your head, but I want to at least mention it: Do not use new and delete. Ever. Instead use make_unique.
TehnicBook *tehnicB = new TehnicBook(blank, blank, false, 0, blank, 0);
becomes
auto tehnicB = make_unique(blank, blank, false, 0, blank, 0);
The point is that now you do not need to manage memory. Managing memory is very difficult and error prone and unnecessary. For example you forgot to clean up tehnicB and litB inside main`. This automatically gets fixed without thinking about it with modern C++.Code Snippets
TehnicBook *tb = new TehnicBook(...);
tb->add(...);
Book *b = tb;
delete tb; //<- tb->head is leakedostream &operator <<(ostream &os, TehnicBook &t){
os <<
os << "-----------------------------\n";
os << "Author: " << p->author << endl;
os << "Title: " << p->title << endl;
os << "Is " << ((p->rented) ? "" : "not ") << "rented" << endl;
os << "Amount of books: " << p->amount << endl;
os << "Language: " << p->language << endl;
os << "Release year: " << p->RlYear << endl;
os << endl;
return os;
}TehnicBook *tehnicB = new TehnicBook(blank, blank, false, 0, blank, 0);auto tehnicB = make_unique<TehnicBook>(blank, blank, false, 0, blank, 0);Context
StackExchange Code Review Q#68219, answer score: 3
Revisions (0)
No revisions yet.