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')

• Graipher: Using a dict with sequential integer indices and only iterating over the dict values means it could just be a list instead (Table._db)...
• Peilonrayz: @Graipher Yes, that could be a good idea. However if you allow removing an item from the DB that means Table._db will have to be List[Optional[T]]. And handling the Optional would be a bit messy.