How to improve operator overloading for my class?-CodePudding

I have started learning C . My teacher gave an assignment. I completed it (some polishing work is left) and everything seems to work but there is redundancy. My major issue is overloading. How to improve overloading in my class. Rather than writing all four functions (two for fstream and two for iostream), is it possible to write only two ? Any other suggestions to improve the code further ?

#include <iostream>
#include <fstream>
#include <string>
#include "../../my_func.hpp"
#define usi unsigned short int

using namespace std;

class book
{
    public:
    usi book_id = 0, price = 0, no_of_pages = 0, year_of_publishing = 0;
    string author_name = "NONE", publisher = "NONE";
    book(usi b_id = 0, usi b_price = 0, usi b_no_of_pages = 0, usi b_year_of_publishing = 0,
         const string& b_author_name = "NONE", const string& b_publisher = "NONE")
    {
        book_id = b_id;
        price = b_price;
        no_of_pages = b_no_of_pages;
        year_of_publishing = b_year_of_publishing;
        author_name = b_author_name;
        publisher = b_publisher;
    }
    friend fstream& operator >> (fstream& is, book& obj);
    friend fstream& operator << (fstream& os, const book& obj);
    friend istream& operator >> (istream &is, book& obj);
    friend ostream& operator << (ostream &os, const book& obj);
};

fstream& operator >> (fstream &is, book& obj)
{
    char ch;
    is >> obj.book_id >> obj.price 
       >> obj.no_of_pages >> obj.year_of_publishing;
    is.ignore(1, '\n'); //To take care of new line character
    getline(is, obj.author_name);
    getline(is, obj.publisher);
    return is;
}

fstream& operator << (fstream &os, const book& obj)
{
    os.operator<<(obj.book_id) << '\n' //calling operator function cuz it works 
               << obj.price << '\n'
               << obj.no_of_pages << '\n'
               << obj.year_of_publishing << '\n';
    os << obj.author_name << '\n'
       << obj.publisher << '\n';
    return os;
}

istream& operator >> (istream &is, book& obj)
{
    is >> obj.book_id >> obj.price 
       >> obj.no_of_pages >> obj.year_of_publishing;
    is.ignore(1, '\n'); //To take care of new line character
    getline(is, obj.author_name);
    getline(is, obj.publisher);
    return is;
}

ostream& operator << (ostream &os, const book& obj)
{
    os << obj.book_id << '\n'
       << obj.price << '\n'
       << obj.no_of_pages << '\n'
       << obj.year_of_publishing << '\n'
       << obj.author_name << '\n'
       << obj.publisher << '\n';
    return os;
}
int main()
{
    string path = ".\\C  _Experiment\\Exp-7\\Files\\Source.txt";
    book b1(12, 3000, 100, 2003, "Lol", "Pew"), b2, b3;
    fstream fio;
    fio.open(path, ios::out | ios::app | ios::in);
    if(fio) fio << b1;
    else cout <<  "error"; 
    fio.seekg(0, ios::beg);
    if(fio) fio >> b2 >> b3;
    cout << b2 << b3;
    fio.close();
    cout << "DONE";
    return 0;
}

CodePudding user response：

You only need two overloads here. ifstream and ofstream inherit from istream and ostream respectively so if you have

friend istream& operator >> (istream &is, book& obj);
friend ostream& operator << (ostream &os, const book& obj);

then those will work with cout and cin, and any fstream or stringstream objects as they also inherit from istream and ostream.

CodePudding user response：

Some other suggestions apart from overloading only for std::istream and std::ostream:

Don't using namespace std;. Prefer to prefix your STL types and algorithms with std::.
Prefer using to typedef, and, in this case, to #define. You could limit the usi alias to the book scope.
Turn book into a struct: if everything within the class book is public. This way, you don't need the operator overloads to be friends of book, since they can directly access the members within a book instance.
Prefer to use member initialization lists, i.e. initialize your member variables right after the list of parameters. You can also provide a default member initialization value at the declaration point; but, in this case, a single {} will zero initialize the usi types. I wouldn't use default parameter values for the constructor in this example.
Consider that, if you provide a custom constructor, the default constructor, copy constructor, move constructor, copy assignment operator, and move assignment operator will be deleted, and thus not available.

[Demo]

#include <iostream>
#include <string>

struct book {
    using usi = unsigned short int;

    usi book_id{};
    usi price{};
    usi no_of_pages{};
    usi year_of_publishing{};
    std::string author_name{"NONE"};
    std::string publisher{"NONE"};

    book() = default;
    book(const book& other) = default;
    book& operator=(const book& other) = default;
    book(book&& other) noexcept = default;
    book& operator=(book& other) noexcept = default;
    ~book() = default;

    book(usi b_id,
         usi b_price,
         usi b_no_of_pages,
         usi b_year_of_publishing,
         const std::string& b_author_name,
         const std::string& b_publisher)
        : book_id{b_id}
        , price{b_price}
        , no_of_pages{b_no_of_pages}
        , year_of_publishing{b_year_of_publishing}
        , author_name{b_author_name}
        , publisher{b_publisher}
    {}
};

std::istream& operator>>(std::istream& is, book& obj) {
    is >> obj.book_id >> obj.price >> obj.no_of_pages >> obj.year_of_publishing;
    is.ignore(1, '\n');  // To take care of new line character
    getline(is, obj.author_name);
    getline(is, obj.publisher);
    return is;
}

std::ostream& operator<<(std::ostream& os, const book& obj) {
    return os 
        << "["
        << obj.book_id << ", "
        << obj.price << ", "
        << obj.no_of_pages << ", "
        << obj.year_of_publishing << ", "
        << obj.author_name << ", "
        << obj.publisher << "]";
}

int main() {
    book b1{ 12, 3000, 100, 2003, "Lol", "Pew" };
    std::cout << "b1: " << b1 << "\n";

    book b2{};
    std::cin >> b2;
    std::cout << "b2: " << b2;
}

Make book an aggregate. You can get rid of a lot of code if you forget about initializing the std::string members to "NONE" (you can always check for empty string at operator<< and output "NONE if needed). You get rid of the custom constructor and, with it, of all the other default constructors and assignment operators.

[Demo]

#include <iostream>
#include <string>

struct book {
    using usi = unsigned short int;

    usi book_id{};
    usi price{};
    usi no_of_pages{};
    usi year_of_publishing{};
    std::string author_name{};
    std::string publisher{};
};

std::istream& operator>>(std::istream& is, book& obj) {
    is >> obj.book_id >> obj.price >> obj.no_of_pages >> obj.year_of_publishing;
    is.ignore(1, '\n');  // To take care of new line character
    getline(is, obj.author_name);
    getline(is, obj.publisher);
    return is;
}

std::ostream& operator<<(std::ostream& os, const book& obj) {
    return os 
        << "["
        << obj.book_id << ", "
        << obj.price << ", "
        << obj.no_of_pages << ", "
        << obj.year_of_publishing << ", "
        << (obj.author_name.empty() ? "NONE" : obj.author_name) << ", "
        << (obj.publisher.empty() ? "NONE" : obj.publisher) << "]";
}

int main() {
    book b1{ 12, 3000, 100, 2003, "Lol", "" };
    std::cout << "b1: " << b1 << "\n";

    book b2{};
    std::cin >> b2;
    std::cout << "b2: " << b2;
}