Bookstore program using standard algorithms

Question

This is the exercise, from C++ Primer 5th edition:

10.32: Rewrite the bookstore problem from § 1.6 (p. 24) using a vector to hold the transactions and various algorithms to do the processing. Use sort with your compareIsbn function from § 10.3.1 (p. 387) to arrange the transactions in order, and then use find and accumulate to do the sum."

Here's my code:

#include <iostream>
#include <vector>
#include <algorithm>
#include <iterator>
#include <numeric>

#include "Sales_item.h"

bool compareIsbnv2(const Sales_item &sd1, const Sales_item &sd2) 
{
     return sd1.isbn() < sd2.isbn();
}


int main()
{
     // get input from user 
     std::istream_iterator<Sales_item> item_iter(std::cin), eof;
     std::ostream_iterator<Sales_item> out_iter(std::cout, "\n");

     // make sure we have data to process
     std::vector<Sales_item> trans(item_iter,eof);
     if (item_iter == eof)
     {
        std::cerr << "no data to process!" << std::endl;
     }
     // sorting vector 

     std::sort(trans.begin(), trans.end(), compareIsbnv2); 

     std::cout << "Summing up data..\n";
     for (auto& s : trans)
         std::cout << s << "\n";

     std::cout << "\n\n";
     //Sales_item sum;
     for (auto beg = trans.begin(); beg != trans.end(); beg++)
     {
         auto it = std::find_if_not(beg, trans.end(), [&](Sales_item& item) 
             {return item.isbn() == beg->isbn(); });
         out_iter = std::accumulate(beg, it, Sales_item(beg->isbn()));
         std::cout << "\n\n";
     }
     return 0;
}

For some reason that I didn't quite understand the original compare isbn function didn't work, so I'm not sure if the result I get is correct:

Input:

0-201-12345-X 3 20.00
0-205-12345-X 5 3.00
0-201-12345-X 2 20.00
0-202-12345-X 10 25.00
0-205-12345-X 10 5.00
0-202-12345-X 5 20.00

Output:

0-201-12345-X 5 100 20

0-201-12345-X 2 40 20

0-202-12345-X 15 350 23.3333

0-202-12345-X 5 100 20

0-205-12345-X 15 65 4.33333

0-205-12345-X 10 50 5

Sales_item.h :

#ifndef SALESITEM_H
// we're here only if SALESITEM_H has not yet been defined 
#define SALESITEM_H

// Definition of Sales_item class and related functions goes here
#include <iostream>
#include <string>

class Sales_item {
    // these declarations are explained section 7.2.1, p. 270 
    // and in chapter 14, pages 557, 558, 561
    friend std::istream& operator>>(std::istream&, Sales_item&);
    friend std::ostream& operator<<(std::ostream&, const Sales_item&);
    friend bool operator<(const Sales_item&, const Sales_item&);
    friend bool
        operator==(const Sales_item&, const Sales_item&);
public:
    // constructors are explained in section 7.1.4, pages 262 - 265
    // default constructor needed to initialize members of built-in type
    Sales_item() = default;
    Sales_item(const std::string& book) : bookNo(book) { }
    Sales_item(std::istream& is) { is >> *this; }
public:
    // operations on Sales_item objects
    // member binary operator: left-hand operand bound to implicit this pointer
    Sales_item& operator+=(const Sales_item&);

    // operations on Sales_item objects
    std::string isbn() const { return bookNo; }
    double avg_price() const;
    // private members as before
private:
    std::string bookNo;      // implicitly initialized to the empty string
    unsigned units_sold = 0; // explicitly initialized
    double revenue = 0.0;
};

// used in chapter 10
inline
bool compareIsbn(const Sales_item& lhs, const Sales_item& rhs)
{
    return lhs.isbn() == rhs.isbn();
}

// nonmember binary operator: must declare a parameter for each operand
Sales_item operator+(const Sales_item&, const Sales_item&);

inline bool
operator==(const Sales_item& lhs, const Sales_item& rhs)
{
    // must be made a friend of Sales_item
    return lhs.units_sold == rhs.units_sold &&
        lhs.revenue == rhs.revenue &&
        lhs.isbn() == rhs.isbn();
}

inline bool
operator!=(const Sales_item& lhs, const Sales_item& rhs)
{
    return !(lhs == rhs); // != defined in terms of operator==
}

// assumes that both objects refer to the same ISBN
Sales_item& Sales_item::operator+=(const Sales_item& rhs)
{
    units_sold += rhs.units_sold;
    revenue += rhs.revenue;
    return *this;
}

// assumes that both objects refer to the same ISBN
Sales_item
operator+(const Sales_item& lhs, const Sales_item& rhs)
{
    Sales_item ret(lhs);  // copy (|lhs|) into a local object that we'll return
    ret += rhs;           // add in the contents of (|rhs|) 
    return ret;           // return (|ret|) by value
}

std::istream&
operator>>(std::istream& in, Sales_item& s)
{
    double price;
    in >> s.bookNo >> s.units_sold >> price;
    // check that the inputs succeeded
    if (in)
        s.revenue = s.units_sold * price;
    else
        s = Sales_item();  // input failed: reset object to default state
    return in;
}

std::ostream&
operator<<(std::ostream& out, const Sales_item& s)
{
    out << s.isbn() << " " << s.units_sold << " "
        << s.revenue << " " << s.avg_price();
    return out;
}

double Sales_item::avg_price() const
{
    if (units_sold)
        return revenue / units_sold;
    else
        return 0;
}
#endif

Bookstore problem from § 1.6 (p. 24) :

We are now ready to solve our original bookstore problem. We need to read a file of sales transactions and produce a report that shows, for each book, the total number of copies sold, the total revenue, and the average sales price. We’ll assume that all the transactions for each ISBN are grouped together in the input. Our program will combine the data for each ISBN in a variable named total. We’ll use a second variable named trans to hold each transaction we read. If trans and total refer to the same ISBN , we’ll update total. Otherwise we’ll print total and reset it using the transaction we just read:

#include <iostream>
#include "Sales_item.h"
int main()
{
Sales_item total; // variable to hold data for the next transaction
// read the first transaction and ensure that there are data to process
if (std::cin >> total) {
Sales_item trans; // variable to hold the running sum
// read and process the remaining transactions
while (std::cin >> trans) {
// if we're still processing the same book
if (total.isbn() == trans.isbn())
total += trans; // update the running total
else {
// print results for the previous book
std::cout << total << std::endl;
total = trans; // total now refers to the next book
}
}
std::cout << total << std::endl; // print the last transaction
} else {
// no input! warn the user
std::cerr << "No data?!" << std::endl;
return -1; // indicate failure
}

Answer 1

About the compare function

For some reason that I didn't quite understand the original compare isbn function didn't work

The compareIsbn() function I see in Sales_item.h compares two items for equality, however sort() has no use for that, it has to know in which order items have to be put. So it needs to be able to compare the order of two items.

I have no idea what you needed exactly in chapter 10, but consider that if you have a function that compares the order of two things, you can use that to get the equality as well, at least if there is a total order:

bool less(const auto& a, const auto& b) {
    return a < b;
}

bool equal(const auto& a, const auto& b) {
    return !less(a, b) && !less(b, a); // a >= b && b >= a → a == b
}

In any case, your compareIsbnv2() is a correct function for std::sort().

Your output contains the same book multiple times

I think the assignment is to have only one line of output for each unique ISBN number. When you have the start and end of a range of identical ISBNs, then you calculate the sum over that, and then skip to the item right after that range. This is simply:

for (auto beg = trans.begin(); beg != trans.end();)
{
     auto it = std::find_if_not(…);
     out_iter = std::accumulate(beg, it, Sales_item(beg->isbn()));
     beg = it;
}

Naming

Make sure you give variables concise but meaningful names. Avoid abbreviations, unless they are very common ones, like i for a loop iterator. Also avoid naming variables after their type. Here is a list of suggested changes to names:

• item_iter -> input
• out_iter -> output
• trans -> transactions
• s -> transaction
• beg -> begin or start
• it -> end, or perhaps end_of_same to make it clear this is the end of the range of identical books.

Incorrect error checking

You are reading the input using std::istream_iterator, which is indeed very helpful here. However, the way you check for errors is not correct. Consider that an error might occur before reaching the end of the input, in which case item_iter might not be equal to eof. It is better to check that std::cin.eof() == true to see if the end of the input has been reached successfully.

What is important after that is that the vector holding the transactions is not empty, so instead of comparing the input iterators, write:

if (transactions.empty())
{
    std::cerr << "No data to process!\n";
    return EXIT_FAILURE;
}

Note also that if you encounter an error, it is usually a good idea to return an error or throw an exception. Luckily, the rest of your code still works fine if there were no transactions on the input. You might also consider this to be a valid input, in which case you shouldn't print an error message at all.

Note that an error can also happen on the output. What if the output is redirected to a file, but there is an error writing data to disk? If you ignore it and return 0, then it might look like your program finished successfully, which is not desirable. Check that std::cout.good() == true at the end of the program, and if not, write an error message to std::cerr and return EXIT_FAILURE.

How it would look in C++23

C++ is continuously evolving, and new features are added to make the life of programmers easier. It looks like the 5th edition of C++ Primer was released in 2012, and thus only covers the standard up to C++11. It's now more than a decade later, and we can do things differently now.

In particular, the ranges library allows us to avoid working with iterators, but operate on ranges instead. Effectively, most algorithms now take a reference to a container as a parameter. Also, there is an easy algorithm to group items in a container based on a predicate using std::views::chunk_by(). For some reason std::accumulate() was never made to work with ranges, but we now have std::ranges::fold_left() and related functions that can be used to do the same. Your code could look like:

#include <algorithm>
#include <iostream>
#include <ranges>
#include <stdexcept>
#include <vector>
…
int main()
{
     std::istream_iterator<Sales_item> input(std::cin), eof;
     std::ostream_iterator<Sales_item> output(std::cout, "\n");

     std::vector transactions(input, eof);
     if (!std::cin.eof())
         throw std::runtime_error("Error reading input");

     std::ranges::sort(transactions, compareIsbnv2);

     for (auto chunk: transactions | std::views::chunk_by(compareIsbn))
         out_iter = std::ranges::fold_left_first(chunk, std::plus<>());

     if (!std::cout.good())
         throw std::runtime_error("Error writing output");
}

Note that it's currently hard to find compilers and standard libraries that have full C++23 support, so the above will probably not compile for you yet.