Conversation
| due_date: datetime | ||
| weekly_fee: float |
There was a problem hiding this comment.
Unused variable. You might want to shift the variables into the __init__ method. With doing so, the referencing using self. throughout the class, e.g., in the borrow_book function should work.
There was a problem hiding this comment.
Consider separation of concerns. The book class represents a collection of books, while a due_data variable is associated with a unique copy of a book. Similarly logic applies to the weekly_fee.
| return book | ||
|
|
||
| def can_borrow(self) -> bool: | ||
| if self._book_type == "Paper": |
There was a problem hiding this comment.
You could consider creating new classes for the different types of books, which all inherit from an abstract book class.
There was a problem hiding this comment.
This would also reduce the number of parameters for the constructor of your objects, since not all variables are necessary for the different types of books
|
|
||
| class BookSerializer: | ||
| def serialize(self, book: Book, format: str): | ||
| def serialize(self, format: str): |
There was a problem hiding this comment.
Separation of concerns.
| reading_credits: int = 0 | ||
|
|
||
| def __init__(self, email, firstname, lastname, mob1, mob2, area_code, landline, country_code): | ||
| def __init__(self, email: str, firstname: str, lastname: str, mob1: str, mob2: str, area_code: str, |
There was a problem hiding this comment.
You could put the information on telephone availability into a separate TelephoneInformation DTO class.
| PAYPAL_DATA_BASE: dict[str, str] = {"amanda1985": "PaSsW0rD", "qwerty": "123456789"} | ||
| PAYPAL_ACCOUNT_BALANCE: dict[str, float] = {"amanda1985": 100.0, "qwerty": 42.0} | ||
|
|
||
| class Paypal: |
There was a problem hiding this comment.
This a nice abstraction and aligns with the existing CreditCard class
You could consider creating a parent class (e.g. PaymentMethod) and using inheritance.
| raise ValueError("Payment information is not set or not valid") | ||
| fee, reading_credits = self.calculate_fee(self.customer) | ||
| is_paid: bool = self._pay_with_credit_card(card, fee) | ||
| if isinstance(pay_method, CreditCard): |
There was a problem hiding this comment.
You could use the actual CreditCard and PayPal classes to process the invoices instead of doing an isInstance check.
4 participants
No description provided.