Object-oriented student library 2

Question

Follow up of Object-oriented student library

Question: How do you refactor this code so that it is pythonic, follows OOP, reads better and is manageable? How can I write name functions and classes better? How do you know which data structure you need to use so as to manage data effectively?

from collections import defaultdict
from datetime import datetime, timedelta

class StudentDataBaseException(Exception): pass
class NoStudent(StudentDataBaseException): pass
class NoBook(StudentDataBaseException): pass

"""To keep of a record of students 
 who have yet to return books and their due dates"""
class CheckedOut:
     loan_period = 10
     fine_per_day = 2

     def __init__(self):
         self.due_dates = {}

     def check_in(self, name):
          due_date = datetime.now() + timedelta(days=self.loan_period)
          self.due_dates[name] = due_date

     def check_out(self, name):
         current_date = datetime.now()
         if current_date > self.due_dates[name]:
             delta = current_date - self.due_dates[name]
             overdue_fine = self.fine_per_day * delta.days
             print("Fine Amount: ", overdue_fine)

# This only contains the title name for now
class BookStatus:
    def __init__(self, title):
        self.title = title

    def __repr__(self):
        return self.title

    def __hash__(self):
        return 0

    def __eq__(self, other):
        return self.title == other

# contains a set of books
class Library:
    record = CheckedOut()

    def __init__(self):
        self.books = set()

    def add_book(self, new_book):
        self.books.add(new_book)

    def display_books(self):
        if self.books:
            print("The books we have made available in our library are:\n")
            for book in self.books:
                print(book)
        else:
            print("Sorry, we have no books available in the library at the moment")

    def lend_book(self, requested_book):
        if requested_book in self.books:
            print(f'''You have now borrowed \"{requested_book}\"''')
            self.books.remove(requested_book)
            return True

        else:
            print(f'''Sorry, \"{requested_book}\" is not there in our library at the moment''')
            return False

# container for students
class StudentDatabase:
    def __init__(self):
        self.books = defaultdict(set)

    def borrow_book(self, name, book, library):
        if library.lend_book(book):
            self.books[name].add(book)
            return True
        return False

    def return_book(self, name, book, library):
        if book not in self.books[name]:
            raise NoBook(f'''\"{name}\" doesn't seem to have borrowed "{book}"''')
            return False
        else:
            library.add_book(book)
            self.books[name].remove(book)
            return True

    def students_with_books(self):
        for name, books in self.books.items():
            if books:
                yield name, books


def borrow_book(library, book_tracking):
    name = input("Student Name: ")
    book = BookStatus(input("Book Title: "))

    if book_tracking.borrow_book(name, book, library):
        library.record.check_in(name)

def return_book(library, book_tracking):
    name = input("Student Name: ")
    returned_book = BookStatus(input("Book Title: "))

    if book_tracking.return_book(name, returned_book, library):
        library.record.check_out(name)


line = "_" * 100
menu = "Library Management System \n\n \
1) Add Book \n \
2) Display all Books \n \
3) Borrow a Book \n \
4) Return a Book \n \
5) Lending Record \n \
6) Exit"

library = Library()
book_tracking = StudentDatabase()

while True:
    print(line)
    print(menu)

    choice = get_valid_choice(min=1, max=6)
    print(line)

    if choice == 1:
        library.add_book(BookStatus(input("Book Title: ")))

    elif choice == 2:
        library.display_books()

    elif choice == 3:
        borrow_book(library, book_tracking)

    elif choice == 4:
        return_book(library, book_tracking)

    elif choice == 5:
        students = tuple(book_tracking.students_with_books())
        if students:
            for name, book in students:
                print(f"{name}: {book}")
        else:
            print("No students have borrowed books at the moment")

    elif choice == 6:
        break

Answer 1

I think the general construction of your code is poor.

1. A book is owned by a library even if the book is on loan.
2. Your code doesn't seem to be able to handle books with the same name.
3. Without running your code it looks like you can steal books by taking one out and returning another.
4. BookStatus is incredibly poorly implemented. I'd instead use dataclasses.
5. You're effectively creating an in-memory database. And so you should design the database, and then the code around that.

Starting with the database design you have three things:

• Book
• Loan
• Person

Yes there is no library, because all books are in your library. Each book can be loaned many times, but only one book can be in a loan. Each person can have multiple loans, but each loan can only be given to one person.

And so your Loan object should have the ID of the book and the person, but neither the book nor the person should have any links to the other database items.

We can then create these objects and tables in Python. I'm using dataclasses (Python 3.6) and typing to build the objects quickly. Since we're already using typing for the dataclass, I decided to make the rest of the program fully typed allowing a static analyzer, such as mypy, to check my code for errors. This allows the base code of:

from dataclasses import dataclass
from typing import Optional, Set, Mapping, TypeVar, Dict, Type, Iterator
from datetime import datetime, timedelta

T = TypeVar('T')


@dataclass
class Book:
    id: int
    name: str


@dataclass
class Person:
    id: int
    name: str


@dataclass
class Loan:
    id: int
    book: Book
    person: Person
    checkout: datetime
    due: datetime
    checkin: Optional[datetime]


class Table(Mapping[int, T]):
    _db: Dict[int, T]

    def __init__(self, type: Type[T]) -> None:
        self._db = {}
        self._type = type

    def __getitem__(self, key: int) -> T:
        return self._db[key]

    def __iter__(self) -> Iterator[T]:
        return iter(self._db)

    def __len__(self) -> int:
        return len(self._db)


books = Table(Book)
people = Table(Person)
loans = Table(Loan)

This then allows us to easily add the additional functionality:

class Table(Mapping[int, T]):
    # Other code
    def add(self, *args, **kwargs) -> None:
        key = len(self)
        self._db[key] = self._type(key, *args, **kwargs)

    def display(self) -> None:
        for value in self.values():
            print(value)


def borrow_book(person: int, book: int, loan_days: int) -> None:
    checkout = datetime.now()
    loans.add(
        books[book],
        people[person],
        checkout,
        checkout + timedelta(days=loan_days),
        None
    )


def return_book(loan: int) -> None:
    loans[loan].checkin = datetime.now()


def display_active_loans() -> None:
    has_active = False
    for loan in loans.values():
        if loan.checkin is not None:
            continue
        has_active = True
        print(f'{loan.id}: {loan.person.name} -> {loan.book.name}')
    if not has_active:
        print('No active loans')

And usage is fairly easy, you just use IDs:

books.add('Title')
books.display()
people.add('Student')
people.display()
borrow_book(0, 0, 10)
display_active_loans()
return_book(0)
display_active_loans()

---

from dataclasses import dataclass
from typing import Optional, Set, Mapping, TypeVar, Dict, Type, Iterator
from datetime import datetime, timedelta

T = TypeVar('T')


@dataclass
class Book:
    id: int
    name: str


@dataclass
class Person:
    id: int
    name: str


@dataclass
class Loan:
    id: int
    book: Book
    person: Person
    checkout: datetime
    due: datetime
    checkin: Optional[datetime]


class Table(Mapping[int, T]):
    _db: Dict[int, T]

    def __init__(self, type: Type[T]) -> None:
        self._db = {}
        self._type = type

    def __getitem__(self, key: int) -> T:
        return self._db[key]

    def __iter__(self) -> Iterator[T]:
        return iter(self._db)

    def __len__(self) -> int:
        return len(self._db)

    def add(self, *args, **kwargs) -> None:
        key = len(self)
        self._db[key] = self._type(key, *args, **kwargs)

    def display(self) -> None:
        for value in self.values():
            print(value)


books = Table(Book)
people = Table(Person)
loans = Table(Loan)


def borrow_book(person: int, book: int, loan_days: int) -> None:
    checkout = datetime.now()
    loans.add(
        books[book],
        people[person],
        checkout,
        checkout + timedelta(days=loan_days),
        None
    )


def return_book(loan: int) -> None:
    loans[loan].checkin = datetime.now()


def display_active_loans() -> None:
    has_active = False
    for loan in loans.values():
        if loan.checkin is not None:
            continue
        has_active = True
        print(f'{loan.id}: {loan.person.name} -> {loan.book.name}')
    if not has_active:
        print('No active loans')