Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Group 5: Pullrequest for refactoring excersice Day2 #13

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 33 additions & 47 deletions library/model/book.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@ class Book:
_book_type: str
duration: int = 0

_duration_constant: int = 60
_booktype_duration: dict = {
"Paper": pages * 3 * _duration_constant,
"Audio": duration,
"Electronic": pages * 5 * _duration_constant
}
_booktype_fee: dict = {
"Paper": 5,
"Audio": 2,
"Electronic": 2
}


def __init__(self, title, authors, publisher, pub_date, genres, pages, isbn, type, duration=0, existing_items=1, borrowed_items=0):
self.title = title
self.authors = authors
Expand All @@ -37,109 +50,82 @@ def __init__(self, title, authors, publisher, pub_date, genres, pages, isbn, typ
self.borrowed_items = borrowed_items

@classmethod
def from_borrowed_book(cls, borrowed_book: "BorrowedBook") -> "Book":
def from_borrowed_book(cls, borrowed_book: "BookCopy") -> "Book":
book = Book(
borrowed_book.title,
borrowed_book.authors,
borrowed_book.publisher,
borrowed_book.publication_date,
borrowed_book.genres,
borrowed_book.pages,
borrowed_book.isbn,
borrowed_book._book_type,
borrowed_book.duration,
borrowed_book.existing_items,
borrowed_book.borrowed_items,
borrowed_book._book_type
borrowed_book.genres
)
return book

def can_borrow(self) -> bool:
if self._book_type == "Paper":
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not a dict here? Seems like a mix of if-else statements and dict lookups.

return self.existing_items - self.borrowed_items > 0
elif self._book_type == "Electronic":
return True
elif self._book_type == "Audio":
elif self._book_type == "Electronic" or self._book_type == "Audio":
return True
else:
raise AttributeError("No such book type...")

def get_approximate_duration(self) -> int:
if self._book_type == "Paper":
return self.pages * 3 * 60
elif self._book_type == "Electronic":
return self.pages * 5 * 60
elif self._book_type == "Audio":
return self.duration
if self._book_type in self._booktype_duration.keys():
return self._booktype_duration[self._book_type]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dict?

else:
raise AttributeError("No such book type...")

def get_weekly_fee(self) -> int:
if self._book_type == "Paper":
return 5
elif self._book_type == "Electronic":
return 2
elif self._book_type == "Audio":
return 2
if(self._book_type in self._booktype_fee.keys()):
return self._booktype_fee[self._book_type]
else:
raise AttributeError("No such book type...")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant, no? A failed dict lookup will raise a KeyError. Maybe you can let it propagate.


def borrow_book(self) -> "BorrowedBook":
def borrow_book(self) -> "BookCopy":
if self.can_borrow():
if self._book_type == "Paper":
self.borrowed_items += 1
LibraryRepository.update_book(self)
borrowed_book = BorrowedBook.from_book(self)
borrowed_book = BookCopy.from_book(self)
borrowed_book.due_date = datetime.now() + timedelta(days=7)
borrowed_book.current_fee = self.get_weekly_fee()
return borrowed_book
raise ValueError("Book cannot be borrowed")

def __eq__(self, other):
"""Overrides the default implementation"""
if isinstance(other, Book) or isinstance(other, BorrowedBook):
if isinstance(other, Book) or isinstance(other, BookCopy):
return self.isbn == other.isbn and self._book_type == other._book_type
return NotImplemented

def __str__(self):
return BookSerializer().serialize(self, "JSON")


class BorrowedBook(Book):
class BookCopy:
due_date: datetime
current_fee: float

@classmethod
def from_book(cls, book: Book) -> "BorrowedBook":
borrowed_book = BorrowedBook(
book.title,
book.authors,
book.publisher,
book.publication_date,
book.genres,
book.pages,
def from_book(cls, book: Book) -> "BookCopy":
borrowed_book = BookCopy(
book.isbn,
book._book_type,
book.duration,
book.existing_items,
book.borrowed_items,
book._book_type
book.genres
)
return borrowed_book

def renew_rental(self) -> "BorrowedBook":
def renew_rental(self) -> "BookCopy":
self.due_date += timedelta(days=7)
self.current_fee += self.get_weekly_fee()
return self

def return_book(self) -> Book:
def return_book(self,book: "Book") -> Book:
if self._book_type == "Paper":
self.borrowed_items -= 1
book = Book.from_borrowed_book(self)
book.borrowed_items -= 1
LibraryRepository.update_book(book)
return book

def __eq__(self, other):
"""Overrides the default implementation"""
if isinstance(other, Book) or isinstance(other, BorrowedBook):
if isinstance(other, Book) or isinstance(other, BookCopy):
return self.isbn == other.isbn and self._book_type == other._book_type
return NotImplemented

Expand Down
31 changes: 17 additions & 14 deletions library/model/user.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from typing import Optional
from library.model.book import Book, BorrowedBook
from library.model.book import Book, BookCopy
from library.model.genre import Genre
from library.persistence.storage import LibraryRepository

Expand All @@ -19,6 +19,11 @@ class User:
mobile_number2: str
reading_credits: int = 0

_reading_credits_dict: dict = {
"HISTORY": 1,
"MEDICINE": 2,
"SOCIOLOGY": 2
}
def __init__(self, email, firstname, lastname, mob1, mob2, area_code, landline, country_code):
self.email = email
self.firstname = firstname
Expand All @@ -32,18 +37,18 @@ def __init__(self, email, firstname, lastname, mob1, mob2, area_code, landline,
self.read_books = []
self.invoices = []

def borrow_book(self, book: Book) -> Optional[BorrowedBook]:
def borrow_book(self, book: Book) -> Optional[BookCopy]:
try:
if book.can_borrow():
borrowed_book = book.borrow_book()
self.borrowed_books.append(borrowed_book)
LibraryRepository.update_user(self)
return borrowed_book
return None
return "Book cannot be borrowed."
except AttributeError:
return None
return "Attribute Error"
except ValueError:
return None
return "Value Error"

def return_books(self, books: list[BorrowedBook]):
from library.payment.invoice import Invoice
Expand All @@ -53,9 +58,11 @@ def return_books(self, books: list[BorrowedBook]):
if borrowed_book in self.borrowed_books:
invoice.add_book(borrowed_book)
self.borrowed_books.remove(borrowed_book)
book = borrowed_book.return_book()
self.read_books.append(book)
LibraryRepository.update_book(book)
book = LibraryRepository.find_book(borrowed_book)
if book is not None:
book = borrowed_book.return_book(book)
self.read_books.append(book)
LibraryRepository.update_book(book)
if len(invoice.books) > 0:
LibraryRepository.create_invoice(invoice)
self.invoices.append(invoice)
Expand All @@ -68,12 +75,8 @@ def get_reading_credits(self, books: list[Book]) -> int:
reading_credits: int = 0
for book in books:
for genre in book.genres:
if genre == Genre.HISTORY:
reading_credits += 1
elif genre == Genre.MEDICINE:
reading_credits += 2
elif genre == Genre.SOCIOLOGY:
reading_credits += 2
if genre in self._reading_credits_dict.keys():
reading_credits += self._reading_credits_dict[genre]
else:
reading_credits += 0
return reading_credits
Expand Down
31 changes: 19 additions & 12 deletions library/payment/invoice.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import uuid

from library.model.user import User
from library.model.book import BorrowedBook, Book
from library.model.book import BookCopy, Book
from library.payment.credit_card import CreditCard
from library.payment.paypal import PAYPAL_ACCOUNT_BALANCE, PAYPAL_DATA_BASE
from library.persistence.storage import LibraryRepository
Expand All @@ -11,16 +11,21 @@
class Invoice:

id: str
books: list[BorrowedBook]
books: list[BookCopy]
customer: User
is_closed: bool = False

price_per_book: float = 3.55
min_books_for_discount: int = 3
discount_per_book: float = 0.5
discount_per_reading_credit: float = 0.5

def __init__(self, user: User):
self.id = str(uuid.uuid4())
self.customer = user
self.books = []

def add_book(self, book: BorrowedBook):
def add_book(self, book: BookCopy):
self.books.append(book)

def __str__(self):
Expand All @@ -38,22 +43,24 @@ def __str__(self):
The invoice is {'' if self.is_closed else 'not'} paid."""

def calculate_fee(self, user: User) -> tuple[float, int]:
price_per_book: float = 3.55
min_books_for_discount: int = 3
discount_per_book: float = 0.5
discount_per_reading_credit: float = 0.5

current_reading_credits = user.reading_credits
reading_credits: int = user.get_reading_credits(
[Book.from_borrowed_book(book) for book in self.books]
)
price: float = len(self.books) * price_per_book
price: float = len(self.books) * self.price_per_book
for book in self.books:
price += book.current_fee
discount_count: int = max(0, len(self.books) - min_books_for_discount)
discount: float = discount_count * discount_per_book
discount += current_reading_credits * discount_per_reading_credit
discount_count: int = max(0, len(self.books) - self.min_books_for_discount)
discount: float = discount_count * self.discount_per_book
discount += current_reading_credits * self.discount_per_reading_credit
final_price = price - discount
Copy link
Collaborator

@denishoornaert denishoornaert Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice but could be broken down even further (e.g., dedicated function/method) :)

if(final_price < 0.0 ):
final_price = 0.0
# round to two digits after the comma
final_price = round(final_price, 2)
return (
round(price - discount if price - discount > 0.0 else 0.0, 2),
final_price,
reading_credits,
)

Expand Down
8 changes: 8 additions & 0 deletions library/persistence/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

if TYPE_CHECKING:
from library.model.book import Book
from library.model.book import BookCopy
from library.model.user import User
from library.model.author import Author
from library.model.publisher import Publisher
Expand Down Expand Up @@ -155,3 +156,10 @@ def delete_invoice(invoice: Invoice):
index = invoices.index(invoice)
if index >= 0:
invoices.remove(invoice)

@staticmethod
def find_book(self,book_copy: "BookCopy") -> "Book":

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def find_book(self,book_copy: "BookCopy") -> "Book":
def find_book(self,book_copy: "BookCopy") -> "Book":

Not sure if you can pass self as input argument to static method. Maybe Maintain an Inventory class, and pass as input arg

for book in self.Books.Book:
if(book.__eq__(book,book_copy)):
Copy link
Collaborator

@denishoornaert denishoornaert Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return book
return None