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 3 - refactoring library class #15

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Group 3 - refactoring library class #15

wants to merge 1 commit into from

Conversation

PtrBld
Copy link
Member

@PtrBld PtrBld commented Nov 13, 2023

No description provided.

crime_audio_books: list[Book] = []
max: Optional[Book] = None

# print banner
Copy link
Collaborator

Choose a reason for hiding this comment

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

Commentary not necessary. Does not say "why"

if b._book_type == "Audio":
if max is not None and max.duration < b.duration:
max = b
# end if
Copy link
Collaborator

Choose a reason for hiding this comment

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

# end if useless as per python

print("********* Longest Crime Audio Book **********")
print("*********************************************")

for b in self.get_books():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we access _books directly?

max = b
# end if
# end if
else:
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be removed. Same as not having an else

if Genre.CRIME in b.genres:
if b._book_type == "Audio":
if max is not None and max.duration < b.duration:
max = b
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is max ever changed from None

def __init__(self, books: list[Book]):
self._books = books

_books: list[Book] = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be placed at the top


class Library():

# Prints crime audio books
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment pollution: the function's name already describes what is does

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants