More efficient way to write this algorithm?

Question

Currently working on a Library simulator assignment. Everything is working fine, but I would like to know something just for the sake of knowing it.

In this program there are 3 classes: Book, Patron, and Library. The library class contains 3 private data members: a vector of pointers to books, a vector to pointers of patron's, and a currentDate int.

The function in question is below:

void Library::incrementCurrentDate()
{
  currentDate++;

  for (int i = 0; i < members.size(); i++)
  {
    vector<Book*> ptr = members.at(i)->getCheckedOutBooks();

     for (int j = 0; j < ptr.size(); j++)
      {
        if (currentDate > ptr.at(j)->getCheckOutLength())
            members.at(i)->amendFine(.10);
      }
  }
} 

the requirements for the function are this:

increment current date; increase each Patron's fines by 10 cents for each overdue Book they have checked out (using amendFine)

The way I have written it above works fine now. As I am just in my first semester of my computer science program, we cannot use anything that we have not covered, which I know is alot. With that being said, would there be a more efficient way to implement this function using more advanced c++ programming methods?

Answer 1

1. Use std::vector if the size is not extremely large.

Pointers always have a cost associated with them because of the indirection involved. Looking up an address and accessing it in memory may not be able to be optimized by the compiler out and will thus involve costs with accessing memory. Memory access is often the bottleneck for performance in systems, so it's best to try to put things near to each other in memory and try to structure your programs so that you access memory the least.

2. Use a database system, like SQL, if the data gets extremely large.

On the other hand, we can forego all of the dirty work and use an established database library or program. Something like MySQL can easily manage a lot of data with a great programming language to access and manage it as well. Certain databases, like PostgreSQL can scale to large sets of data. Getting familiar with it can also be quite helpful. Even some mobile apps might use MySQL for Android, for example.

3. Use the modern C++11 or greater for loop iteration syntax.

The current for loop syntax is quite opaque and might have a lot of cruft. C++11 introduced a cleaner for loop syntax to iterate across standard library containers like map or vector. Use: for(auto it : vector_name). If you need to modify each one, use a reference qualifier for the it--the iterator.

4. Use pre-increment syntax for possibly minimal speedup.

++i and i++ are slightly different. ++i just directly modifies the object where it appears in an expression before it continues evaluating it. i++ creates a copy of the object, returns it, and increments the original. Creating a copy of a value or object has a cost in C++, so avoiding this can be helpful in certain cases and it is a good convention to do this anyways.

5. Pass by const &. Not by just regular reference.

Function arguments are passed by value by default in C++. This means that C++ just makes a copy of the object. However, when there are mutations applied repeatedly to an object, like, say, using a function to change the value of an integer over time, you may want to pass by reference. References basically pass the "real" object, meaning that any changes you make to the reference are done on the "real" object.

Now, why pass a non-modifiable object? Because it can lead to better optimizations. Passing by constant reference allows the compiler to make stronger assumptions about your code (e.g. because the reference cannot change within the course of the program, referring to the same reference multiple times in the function doesn't require the value of the argument to be reloaded over again because it shouldn't change while inside of a function).

6. Use a std::unique_ptr or std::shared_ptr.

Smart pointers are also a nice feature that was introduced with C++11, and involves pointers that automatically deallocate themselves by attaching their lifetime to scope. In other words, no need to use new or delete--just create the pointers and you shouldn't have to keep track of when to release memory. This can get complicated in certain situations but in general, using smart pointers leads to better safety and less change of having memory management problems, which is why they were inducted into the standard library in the first place.

Answer 2

There are a couple of questions to answer here I think. The first being: can this algorithm be more efficient? And the other being: can my implementation of the algorithm in c++ be more efficient?

To the first question, I would answer no. Based on the problem, it sounds to me like you have no further information that would allow you to do any better than O(n^2).

As mentioned in the comments, you could iterate over every person and sort their books by due date. In practice this could save some time, but in theory book lookup would still be linear time, O(n). Plus you add the overhead of sorting making your algorithm now O(mnlog(n)) where m is the number of patrons and n is the number of books. If you know you have few patrons with many books each, then sorting could be beneficial. If you have many patrons with few books, it would be much less beneficial.

As for the second question: there are a few small tweaks (and a few large tweaks) that could make your code more efficient, although I would argue that a vast majority of the time they would not be necessary. One major thing I notice is that you recreate a vector object on every iteration of you first for loop. By doing this you are creating unnecessary overhead. Try instead this pseudo-code:

currentDate++;
vector<Book*> ptr = members.at(i)->getCheckedOutBooks();
for(....)

Another optimization that could be a large overhaul would be to drop the Vector library. A vector in c++ has the ability to be resized on the fly as well as other unnecessary overhead (for your task). Simply using an array would be more memory efficient, although equivalently time efficient.

You mentioned being in your first semester, so you probably have not been introduced to Big O notation yet.

Answer 3

if that is the only operation you are optimizing then keep a vector of tuple <int, Book *, Patron * > sorted by the int representing due date of evey checked out Book Then just iterate until the due date is greater than the current date appying fines the the associated Patron.

Answer 4

If you have n checked out books, m of which are overdue, your algorithm takes O(n) time to add the fines. This is because your data structure stores information like this

member -> list(checked out books)
book -> check-out length // presumably the due date for returning the book

If in addition to your members collection you also store the following information:

check-out length -> list(checked out books with that due date)
book -> member who checked it out

then you can use a sorted tree that stores all checked-out books by their due date to look up all overdue books in O(log n). Thus the total asymptotic run-time of your algorithm would reduce from O(n) to O(log n + m).

Answer 5

You might consider replacing the vector with an std::map container. Maps are stored as sorted trees. If you define a comparator function comparing checkout length (or more likely "expired date") you do not need to scan the entire list every time.

A more complicated solution would store all books in a single tree of pointers sorted by their expiration time. Then you wouldn't have to iterate over members at all. Instead iterate over books until you find a book that has not expired yet.

This is more complicated because now adding/removing books for each member (or even iterating over all books a member owns) is more difficult and may require maintaining a vector of pointers for each user as your current approach (in addition to the global book map).

Answer 6

It's been a while since I've used C++, but almost always the standard libraries will be faster than your own implementation. For your reference, check out the standard functions that are associated with std::vector (this site is extremely useful).

You might be able to slim down the ptr.size() through some other filtering logic so you don't have to iterate over people who don't have late books (maybe some sorting on books and their due dates?)

Answer 7

Right now you're amend fines in O(n) (n being getCheckOutLength().size()) but you can do it in O(log(n)) since you need just number of late books and not their objects for fining, if you have that number then you multiply it by .01 and use one amend fine function to do it all.

So here is the way I suggest:
If you keep getCheckedOutBooks() sorted by their getCheckOutLength() in a vector then you can find which dates are more than curDate by finding std::upper_bound in the vector that gives you the first element that is greater than the currentDate, so from that element index to the end of vector is number of book who should be fined, here is the code:

int checkedDateComparator(Patron & leftHand, Patron & rightHand){
    return leftHand.getCheckedOutLength() < rightHand.getCheckOutLength();  
}
bool operator==(Patron & a, Patron & b){
    return a.getCheckedOutLength() < b.getCheckOutLength();
}
void Library::incrementCurrentDate()
{
    currentDate++;

    for (int i = 0; i < members.size(); i++)
    {
        vector<Book*> ptr = members.at(i)->getCheckedOutBooks();
        Book dummy; //dummy used for find the fines 
        dummy.setCheckedOutLength(currentDate);
        int overdue = ptr.end() - upper_bound(ptr.begin(), ptr.end(), dummmy, checkedDateComparator);
        members.at(i)->amendFine(overdue* .01);
   }
} 

Answer 8

Let's take a step back and look at the requirements. When you go to the library and have some possibly-late checked-out books, you probably ask the librarian what you owe. The librarian looks up your account and tells you. It is at that point that you should be calculating the fees. What you're doing now is recalculating the fees every midnight (I'm assuming). That's the part that's inefficient.

Let's instead have this use-case:

1. Librarian attempts to check out a patron's books
2. System calculates fees
3. Patron pays any outstanding fees
4. Librarian checks out the books

The relevant part for your question would be step #2. Here's pseudocode:

float CalculateFees(Patron patron)
{
    float result = 0;
    foreach(checkedOutBook in patron.GetCheckedOutBooks())
    {
        result += CalculateFee(checkedOutBook.CheckOutDate(), today);
    }
    return result;
}

float CalculateFee(Date checkOutDate, Date today)
{
    return (today.Day() - checkOutDate.Day()) * 0.10;
}

The whole use-case could be as simple as:

void AttemptCheckout(Patron patron, BookList books)
{
    float fees = CalculateFees(patron);
    if(fees == 0 || (fees > 0 && PatronPaysFees(patron, fees)))
    {
        Checkout(patron, books);
    }
    else
    {
        RejectCheckout(patron);
    }
}

I've written this in a way that makes it easy to change the fee formula. Some types of materials accrue fines differently than other types of materials. Fines may be capped at a certain amount.