I have never written program in file handling. Help me improve this program.
book.h
#ifndef BOOK_H_
#define BOOK_H_
#include <string>
class Book
{
std::string book_num;
std::string book_name;
std::string author_name;
public:
Book() = default;
Book(const Book&) = delete;
Book &operator=(const Book&) = delete;
~Book() = default;
void new_entry();
void show_book(std::string&, std::string&, std::string&);
std::string get_book_num() const;
void show_record();
};
#endif
book.cpp
#include <iostream>
#include <iomanip>
#include <fstream>
#include "book.h"
void Book::new_entry()
{
std::cin.ignore();
std::cout << "Enter Book Number : ";
std::getline(std::cin, book_num);
std::cout << "\nEnter Book Name : ";
std::getline(std::cin, book_name);
std::cout << "\nEnter Author Name : ";
std::getline(std::cin, author_name);
std::fstream fp;
fp.open("Books.dat", std::ios::out | std::ios::app);
if (fp)
{
fp << book_num << " " << book_name << " " << author_name << '\n';
}
fp.close();
std::cout << "Entry Successfull!!\n";
}
void Book::show_book(std::string& b_num, std::string& b_name, std::string& a_name)
{
std::cout << "Book Number :" << std::setw(10) << b_num << '\n';
std::cout << "Book Name : " << std::setw(10) << b_name << '\n';
std::cout << "Author Name :" << std::setw(10) << a_name << '\n';
}
std::string Book::get_book_num() const
{
return book_num;
}
void Book::show_record()
{
std::fstream fp;
std::string record;
fp.open("Books.dat", std::ios::in);
if (!fp)
{
std::cerr << "File could not be opened\n";
return;
}
else
{
while (fp >> book_num >> book_name >> author_name)
{
if (fp.eof())
{
break;
}
else
{
std::cout << book_num << std::setw(50) << book_name << std::setw(50) << author_name << '\n';
}
}
}
fp.close();
}
student.h
#ifndef STUDENT_H_
#define STUDENT_H_
class Student
{
std::string roll_num;
std::string stu_name;
std::string issued_book_num;
unsigned int token;
public:
Student() = default;
Student(const Student&) = delete;
Student &operator=(const Student&) = delete;
~Student() = default;
void new_entry();
void show_stu(std::string&, std::string&, unsigned int, std::string&);
void reset_issued_book_num();
void reset_token();
void show_record();
};
#endif
student.cpp
#include <iostream>
#include <iomanip>
#include <fstream>
#include <sstream>
#include "student.h"
void Student::new_entry()
{
std::cin.ignore();
std::cout << "Enter Roll Number : ";
std::getline(std::cin, roll_num);
std::cout << "\nEnter Student Name : ";
std::getline(std::cin, stu_name);
token = 0;
issued_book_num = "No";
std::fstream fp;
fp.open("Students.dat", std::ios::out | std::ios::app);
if (fp)
{
fp << roll_num << " " << stu_name << " " << token << " " << issued_book_num << '\n';
}
fp.close();
std::cout << "Entry Successfull!!\n";
}
void Student::show_stu(std::string& r_num, std::string& s_name, unsigned int tkn, std::string& issued_b_num)
{
std::cout << "Roll Number : " << std::setw(10) << r_num << '\n';
std::cout << "Student Name :" << std::setw(10) << s_name << '\n';
std::cout << "Books issued :" << std::setw(10) << tkn << '\n';
if (tkn == 1)
{
std::cout << "Book Number :" << std::setw(10) << issued_b_num << '\n';
}
std::cout << '\n';
}
void Student::reset_issued_book_num()
{
issued_book_num = "";
}
void Student::reset_token()
{
token = 0;
}
void Student::show_record()
{
std::fstream fp;
std::string record;
fp.open("Students.dat", std::ios::in);
if (!fp)
{
std::cerr << "File could not be opened\n";
return;
}
else
{
std::string line;
while (std::getline(fp, line))
{
std::stringstream ss(line);
ss >> roll_num >> stu_name >> token >> issued_book_num;
std::getline(ss, line);
if (fp.eof())
{
break;
}
else
{
std::cout << roll_num << "\t\t\t" << stu_name << "\t\t\t" << token << "\t\t\t" << issued_book_num << '\n';
}
}
}
fp.close();
}
library.h
#ifndef LIBRARY_H_
#define LIBRARY_H_
#include <fstream>
#include <string>
#include "book.h"
#include "student.h"
class Library : public Book, public Student
{
std::fstream fp;
//std::ifstream ifs;
Book book;
Student student;
public:
Library() = default;
Library(const Library&) = delete;
Library &operator=(const Library&) = delete;
~Library() = default;
void enter_book();
void enter_stu();
void display_book(const std::string&);
void display_stu(const std::string&);
void delete_book();
void delete_stu(std::string&);
void display_all_stu();
void display_all_book();
void book_issue();
void book_deposit();
void menu();
};
#endif
library.cpp
#include <iostream>
#include <string>
#include <fstream>
#include <boost/algorithm/string.hpp> //case insensitive compare
#include <stdlib.h>
#include <iomanip>
#include <sstream>
#include "library.h"
#include "book.h"
#include "student.h"
void Library::enter_book()
{
char choice;
do
{
book.new_entry();
std::cout << "Do you want to add more records? (Y/N)\n";
std::cin >> choice;
}while(choice == 'y' || choice == 'Y');
fp.close();
}
void Library::enter_stu()
{
char choice;
do
{
student.new_entry();
std::cout << "Do you want to add more records? (Y/N)\n";
std::cin >> choice;
}while(choice == 'y' || choice == 'Y');
fp.close();
}
void Library::display_book(const std::string& num)
{
std::cout << "---------------Book Details-------------\n";
bool exist = false;
std::string b_num, b_name, a_name;
fp.open("Books.dat", std::ios::in);
while(fp >> b_num >> b_name >> a_name)
{
if (boost::iequals(b_num, num))
{
book.show_book(b_num, b_name, a_name);
exist = true;
}
}
fp.close();
if (!exist)
{
std::cerr << "Book does not exist\n";
}
}
void Library::display_stu(const std::string& num)
{
std::cout << "--------------Student Details-----------------\n";
bool exist = false;
std::string r_num, s_name, issued_b_num;
std::string line, str;
unsigned int tkn;
fp.open("Students.dat", std::ios::in);
while(std::getline(fp, line))
{
std::stringstream ss(line);
ss >> r_num >> s_name >> tkn >> issued_b_num;
std::getline(ss, str);
if (boost::iequals(r_num, num))
{
student.show_stu(r_num, s_name, tkn, issued_b_num);
exist = true;
}
}
fp.close();
if (!exist)
{
std::cerr << "Student does not exist\n";
}
}
void Library::delete_book()
{
std::string num;
std::cout << "Delete Book\n";
std::cout << "Enter number of the book you want to delete : ";
std::cin.ignore();
std::getline(std::cin, num);
std::cout << '\n';
fp.open("Books.dat", std::ios::in | std::ios::out);
std::fstream fp1;
fp1.open("Temp.dat", std::ios::out);
fp.seekg(0, std::ios::beg);
std::string b_num, b_name, a_name;
while(fp >> b_num >> b_name >> a_name)
{
if (!boost::iequals(b_num, num))
{
fp1 << b_num << " " << b_name << " " << a_name << '\n';
}
}
fp1.close();
fp.close();
std::remove("Books.dat");
std::rename("Temp.dat", "Books.dat");
}
void Library::delete_stu(std::string& num)
{
fp.open("Students.dat", std::ios::in | std::ios::out);
std::fstream fp1;
fp1.open("Temp.dat", std::ios::out);
fp.seekg(0, std::ios::beg);
std::string r_num, s_name, issued_b_num;
int tkn;
while(fp >> r_num >> s_name >> tkn >> issued_b_num)
{
if (!boost::iequals(r_num, num))
{
fp1 << r_num << " " << s_name << " " << " " << tkn << " " << issued_b_num << '\n';
}
}
fp1.close();
fp.close();
std::remove("Students.dat");
std::rename("Temp.dat", "Students.dat");
}
void Library::display_all_stu()
{
std::cout << " ---------------Students List ----------------\n";
std::cout << "Roll No." << "\t\t\t" << "Name" << "\t\t\t" << "Book Issued" << "\t\t\t" << "Issued Book No.\n";
student.show_record();
fp.close();
}
void Library::display_all_book()
{
std::cout << "-----------------Books List------------------\n";
std::cout << "Book No." << std::setw(50) << "Name" << std::setw(50) << "Author\n";
book.show_record();
}
void Library::book_issue()
{
std::string r_num, b_num; // roll num and book num
std::string roll_n, s_name, issued_b_num;
std::string book_n, b_name, a_name;
unsigned int tkn;
std::string line, str;
std::fstream fp1;
bool found_stu = false;
bool found_book = false;
std::cout << "-----------------Book Issue--------------------\n";
std::cout << "Enter student's roll no. : ";
std::cin.ignore();
std::getline(std::cin, r_num);
std::cout << '\n';
fp.open("Students.dat", std::ios::in | std::ios::out);
fp1.open("Books.dat", std::ios::in | std::ios::out);
int oldPos = fp.tellg();
while (std::getline(fp, line) && !found_stu)
{
std::stringstream ss(line);
ss >> roll_n >> s_name >> tkn >> issued_b_num;
std::getline(ss, line);
if (boost::iequals(roll_n, r_num))
{
found_stu = true;
if (tkn == 0)
{
std::cout << "Enter Book No. : ";
std::getline(std::cin, b_num);
while (fp1 >> book_n >> b_name >> a_name && !found_book)
{
if (boost::iequals(book_n, b_num))
{
book.show_book(book_n, b_name, a_name);
found_book = true;
tkn = 1;
student.reset_issued_book_num();
issued_b_num = book_n;
fp.seekg(oldPos);
fp << roll_n << " " << s_name << " " << tkn << " " << issued_b_num << '\n';
std::cout << "Book Issued Successfully\n";
break;
}
}
if (!found_book)
{
std::cerr << "Book does not exist\n";
}
}
}
}
if (!found_stu)
{
std::cout << "Student record does not exist\n";
}
fp.close();
fp1.close();
}
void Library::book_deposit()
{
std::string r_num, b_num; // roll num and book num
std::string roll_n, s_name, issued_b_num;
std::string book_n, b_name, a_name;
unsigned int tkn;
std::string line, str;
std::fstream fp1;
int days, fine;
bool found_stu = false;
bool found_book = false;
std::cout << "-----------------Book Deposit---------------------\n";
std::cout << "Enter student's roll no. : ";
std::cin.ignore();
std::getline(std::cin, r_num);
std::cout << '\n';
fp.open("Students.dat", std::ios::in | std::ios::out);
fp1.open("Books.dat", std::ios::in | std::ios::out);
while (std::getline(fp, line) && !found_stu)
{
std::stringstream ss(line);
ss >> roll_n >> s_name >> tkn >> issued_b_num;
std::cout << "IBN " << issued_b_num << '\n';
std::getline(ss, line);
if (boost::iequals(roll_n, r_num))
{
found_stu = true;
if (tkn == 1)
{
while (fp1 >> book_n >> b_name >> a_name && !found_book)
{
if (boost::iequals(book_n, issued_b_num))
{
book.show_book(book_n, b_name, a_name);
found_book = true;
std::cout << "Book deposited in no. of days : ";
std::cin >> days;
if (days > 15)
{
fine = days - 15;
std::cout << "Fine has to be deposited : " << fine << '\n';
}
student.reset_token();
int pos = -1 * sizeof("Students.dat");
fp.seekp(pos, std::ios::cur);
fp << roll_n << s_name << tkn << issued_b_num;
std::cout << "Book Deposited Successfully\n";
}
}
if (!found_book)
{
std::cerr << "Book does not exist\n";
}
}
}
}
if (!found_stu)
{
std::cout << "Student record does not exist\n";
}
fp.close();
fp1.close();
}
void Library::menu()
{
int choice;
std::cout << "Menu\n";
std::cout << "1. Create Student Record\n";
std::cout << "2. Display all Students Record\n";
std::cout << "3. Display Specific Student Record\n";
std::cout << "4. Delete Student Record\n";
std::cout << "5. Enter Book Record\n";
std::cout << "6. Display all Books\n";
std::cout << "7. Display Specific Book\n";
std::cout << "8. Delete Book\n";
std::cout << "9. Back to Main Menu\n";
std::cout << "Enter your choice\n";
std::cin >> choice;
switch(choice)
{
case 1: enter_stu();
break;
case 2: display_all_stu();
break;
case 3: {
std::string num;
std::cout << "Enter Roll No.\n";
std::cin.ignore();
std::getline(std::cin, num);
display_stu(num);
}
break;
case 4: {
std::string num;
std::cout << "Delete Student\n";
std::cout << "Enter roll number of the student you want to delete : ";
std::cin.ignore();
std::getline(std::cin, num);
delete_stu(num);
}
break;
case 5: enter_book();
break;
case 6: display_all_book();
break;
case 7: {
std::string num;
std::cout << "Enter Book No.\n";
std::cin.ignore();
std::getline(std::cin, num);
display_book(num);
}
break;
case 8: delete_book();
break;
case 10: return;
default: std::cout << "Please enter any of these choices\n";
break;
}
}
main.cpp
#include <iostream>
#include <stdlib.h>
#include "library.h"
int main()
{
Library lib;
char choice;
do
{
std::cout << "1. Book Issue\n";
std::cout << "2. Book Deposit\n";
std::cout << "3. Menu\n";
std::cout << "4. Exit\n";
std::cout << "Enter your option\n";
std::cin >> choice;
switch(choice)
{
case '1': lib.book_issue();
break;
case '2': lib.book_deposit();
break;
case '3': lib.menu();
break;
case '4': exit(0) ;
default: std::cout << "Enter one of these choice\n";
}
} while(choice != '4');
}
3 Answers 3
For feedback on your file handling, see the bottom of this post. First, I want to tackle a much larger issue:
None of your classes actually represent objects.
The purpose of object-oriented programming is to model real-world objects and their relationships in code, as closely as possible. Objects have a state and a set of actions that can be taken to modify that state.
Your classes contain sets of related functionality, but none of them actually maintain any sort of state. You could easily remove all member variables of Library
, Student
and Book
and use local variables instead, and nothing would change about your functionality. I'll show you a rewritten Book
module as an example of this:
Book.h
#ifndef BOOK_H_
#define BOOK_H_
#include <string>
// Book is no longer a class - just a namespace, a collection of related functions.
namespace Book
{
void new_entry();
void show_book(std::string&, std::string&, std::string&);
// get_book_num can be removed, as it is not actually called anywhere.
// std::string get_book_num() const;
void show_record();
}
#endif
Book.cpp
#include <iostream>
#include <iomanip>
#include <fstream>
#include "Book.h"
namespace Book
{
void new_entry() {
// These variables are initialized and consumed entirely within this function,
// so there is no reason to keep track of their values outside of this function.
std::string book_num, book_name, author_name;
std::cin.ignore();
std::cout << "Enter Book Number : ";
std::getline(std::cin, book_num);
std::cout << "\nEnter Book Name : ";
std::getline(std::cin, book_name);
std::cout << "\nEnter Author Name : ";
std::getline(std::cin, author_name);
std::fstream fp;
fp.open("Books.dat", std::ios::out | std::ios::app);
if (fp)
{
fp << book_num << " " << book_name << " " << author_name << '\n';
}
fp.close();
std::cout << "Entry Successfull!!\n";
}
void show_book(std::string& b_num, std::string& b_name, std::string& a_name)
{
std::cout << "Book Number :" << std::setw(10) << b_num << '\n';
std::cout << "Book Name : " << std::setw(10) << b_name << '\n';
std::cout << "Author Name :" << std::setw(10) << a_name << '\n';
}
void show_record()
{
std::fstream fp;
std::string record;
fp.open("Books.dat", std::ios::in);
if (!fp)
{
std::cerr << "File could not be opened\n";
return;
}
else
{
std::string book_num, book_name, author_name;
while (fp >> book_num >> book_name >> author_name)
{
if (fp.eof())
{
break;
}
else
{
std::cout << book_num << std::setw(50) << book_name << std::setw(50) << author_name << '\n';
}
}
}
fp.close();
}
}
The only other thing to change now is, everywhere you were previously calling book.new_entry()
or similar functions, change to Book::new_entry()
. This works because our Book
class did not actually represent a book - it simply contained functions that knew how to read and write information about a book to/from a file.
Your program as-is is simply a wrapper around a file. Everything is read-from or written-to the file directly. Since your program does not keep any information in memory from one function call to the next, it has no state to manage, so an object-oriented solution makes no sense.
If you want to practice with object-oriented programming, I would recommend a slightly different exercise. Write a program that begins by reading in a list of Books and Students from their respective files, and keeps this information in memory. Allow the user to view, add, and remove records as they wish, and then write this data back to the files when the user exits the program. Apart from the very beginning and very end of the program, you are not allowed to touch the file.
This exercise should give you much more practice with designing objects and maintaining their state. You will need to keep track of all the Books and Students you have read from the file, as well as the ones the user has entered. You will need to design a way for the user to look up a Book or Student from those that are currently in memory. You will need to decide which objects have what responsibility, and how to store their data.
I predict that, if you attempt the above exercise and post a solution to that, you will get much more helpful feedback about object-oriented techniques.
File handling
Ignoring the issues discussed above, your file handling code is mostly fine. You can improve it in the following ways:
- Either use a different filestream object for each function, or don't call
fp.close()
. Looking at yourLibrary
class, thefstream
object is a member variable, which implies it maintains its state (i.e the file it represents) across many function calls. However, in your implementation, you callfp.open()
andfp.close()
manually every time you want to use the file. You should either open thefstream
in theLibrary
constructor and leave it that way, or create a newfstream
local variable for each function. Use RAII techniques. Your file stream objects should be initialized and ready-to-use the moment they are created. You can accomplish this by specifying the file to open in the
fstream
constructor, rather than callingfstream::open
manually. Example inBook::new_entry
:std::fstream fp( "Books.dat", std::ios::out | std::ios::app ); // Now fp is ready to be used - no need to call 'open()' manually. if (fp) ...
Additionally, the
fstream
destructor will automatically close a file stream if it is currently open. Therefore, there is no reason to callfp.close()
manually unless you are trying to reuse thefstream
object by opening a different file. If you remove the member variablefp
inLibrary
and only use localfstreams
, then you can safely remove all calls tofp.close()
.Don't check
fstream::eof()
manually. Looking atBook::show_record
, you have the following code:while (fp >> book_num >> book_name >> author_name) { if (fp.eof()) { break; } // Process data }
The
eofbit
for anfstream
is set when end-of-file is reached for that stream, i.e once you have read the last piece of data in the stream. When you read the last line in thiswhile
-loop, theeofbit
is set. By checkingfp.eof()
and breaking out of the loop if it is set, you have actually introduced a bug where your program will never process the last line of this file (you read it in,eofbit
gets set, and you check foreof
before processing the line).fstream::operator>>
will return anfstream
that evaluates tofalse
if itseofbit
is set, so you can remove this entireif (fp.eof())
check. Instead, let thewhile
-loop condition evaluate tofalse
naturally ifeofbit
gets set. This way, you process every single line of the file, and then thewhile
-loop stops running when you try to read in data past the end of the file.while (fp >> book_num >> book_name >> author_name) { // Process data normally, no need to check for eof. // If any of the above data fails to read in, we will just skip to the end of the while-loop. }
Misc
Explain your rationale for using
setw
. Looking atBook::show_book
, it is odd to me that you choose to usesetw(10)
. If the book name and author name are ten characters or shorter, this will ensure they line up with the book number. However, if they are longer than ten characters, then they will wrap to the right and throw off your carefully chosen alignment of 10.Should all book information line up at the ten character mark? Is this incorrect behavior, and you actually wanted to truncate book titles that are longer than ten characters? A comment or two about why you chose a value of 10 would be nice to see here.
As far as file handling goes this seems pretty basic so not much to say about that. There are some minor points to note though.
Consider moving private members below public ones. Most users of libraries care about the interface not the implementation.
I think you can get away with not defaulting/deleting your ctors/dtors here. You don't use any of them so you might as well drop them and let the compiler handle it.
In my opinion you should not omit parameter names from your interface as it makes it harder to understand.
Your variable names in general could use some work to make them more telling.
E.g. what exactly is token and what does it do? Why is it compared to 0/1?
fp
is another name that should probably be changed so people don't think of C-style file pointers when they read it. Even something simple asinstream
oroutstream
(depending on use case) would be more indicative of what it is and does.
successfull
: A common typo but it's written without the 2ndl
.Hardcoding filenames all over the program is not the best of ideas. What if you want to change them later on? At least have all the names in one place or pass them in when you create the objects.
Instead of writing
std::fstream foo;
andfoo.open("bar.baz", std::ios::out);
you can writestd::ofstream
orstd::ifstream
.
E.gstd::ifstream infile{"foo.bar};
.I really don't understand why
Library
derives from eitherStudent
orBook
. Not only is it unneccessary but also makes no sense.
I don't know where to begin... this would be very very long answer. Why would Library class inherit from Book or even Student? Library is not a Student. You should use students inside a library as a member field, or something, not inherit from it. I suggest you to start all over, and read some C++ concepts. This is not about file reading and writing, this is about code hierarchy and basic understanding of C++ (OOP) code and practices. Read about classes, inheritance, encapsulations and other concepts. Try following some tutorial. I know that this is not the answer you would want to hear, but maybe somebody will give you a better answer. Best regards and keep learning.