Return to Answer

Minor fixes

Source Link

edit approved Nov 26, 2017 at 7:41

Your design doesn't follow best practice. Your Library class should create a library when you use __init__, rather than a book. And so I'd split your code in half, create two classes; one for books, the other for the library.
You shouldn't mutate data on LibaryLibrary. This is as it mutates it on the class, rather than on the instance. And so if you make a new instance, then it will be tied to all the other instances.
When making the book class. This can significantly be simplified with collections.namedtuple. Such as:
```
 Book = namedtuple('Book', 'name author ISBN price')
```

from collections import namedtuple


class Library(object):
    def __init__(self):
        self.books = []

    def addBook(self, book):
        self.books.append(book)

    def searchBookISBN(self, ISBN):
        for book in self.books:
            if book.ISBN == ISBN:
                return book

    def searchBookAuthor(self, author):
        written_by_author = []
        for book in self.books:
            if book.author == author:
                written_by_author.append(book)
        return written_by_author

    def searchUnderPrice(self, price):
        books_under_price = []
        for book in self.books:
            if book.price < price:
                books_under_price.append(book)
        return books_under_price


Book = namedtuple('Book', 'name author ISBN price')

library = Library()
library.addBook(Book('Geometry', 'Jeff Potter', '0596805888', 22))
library.addBook(Book('Math', 'George Harr', '0594805888', 15))
library.addBook(Book('English', 'James Odd', '0596225888', 10))
library.addBook(Book('Physics', 'Jeff Potter', '0597884512', 18))
print(library.searchBookISBN('0594805888'))
print(library.searchBookAuthor('George Harr'))
print(library.searchUnderPrice(20))

Your design doesn't follow best practice. Your Library class should create a library when you use __init__, rather than a book. And so I'd split your code in half, create two classes; one for books, the other for the library.
You shouldn't mutate data on Libary. This is as it mutates it on the class, rather than on the instance. And so if you make a new instance, then it will be tied to all the other instances.
When making the book class. This can significantly be simplified with collections.namedtuple. Such as:
```
 Book = namedtuple('Book', 'name author ISBN price')
```

from collections import namedtuple


class Library(object):
    def __init__(self):
        self.books = []

    def addBook(self, book):
        self.books.append(book)

    def searchBookISBN(self, ISBN):
        for book in self.books:
            if book.ISBN == ISBN:
                return book

    def searchBookAuthor(self, author):
        written_by_author
        for book in self.books:
            if book.author == author:
                written_by_author.append(book)
        return written_by_author

    def searchUnderPrice(self, price):
        books_under_price = []
        for book in self.books:
            if book.price < price:
                books_under_price.append(book)
        return books_under_price


Book = namedtuple('Book', 'name author ISBN price')

library = Library()
library.addBook(Book('Geometry', 'Jeff Potter', '0596805888', 22))
library.addBook(Book('Math', 'George Harr', '0594805888', 15))
library.addBook(Book('English', 'James Odd', '0596225888', 10))
library.addBook(Book('Physics', 'Jeff Potter', '0597884512', 18))
print(library.searchBookISBN('0594805888'))
print(library.searchBookAuthor('George Harr'))
print(library.searchUnderPrice(20))

Your design doesn't follow best practice. Your Library class should create a library when you use __init__, rather than a book. And so I'd split your code in half, create two classes; one for books, the other for the library.
You shouldn't mutate data on Library. This is as it mutates it on the class, rather than on the instance. And so if you make a new instance, then it will be tied to all the other instances.
When making the book class. This can significantly be simplified with collections.namedtuple. Such as:
```
 Book = namedtuple('Book', 'name author ISBN price')
```

from collections import namedtuple


class Library(object):
    def __init__(self):
        self.books = []

    def addBook(self, book):
        self.books.append(book)

    def searchBookISBN(self, ISBN):
        for book in self.books:
            if book.ISBN == ISBN:
                return book

    def searchBookAuthor(self, author):
        written_by_author = []
        for book in self.books:
            if book.author == author:
                written_by_author.append(book)
        return written_by_author

    def searchUnderPrice(self, price):
        books_under_price = []
        for book in self.books:
            if book.price < price:
                books_under_price.append(book)
        return books_under_price


Book = namedtuple('Book', 'name author ISBN price')

library = Library()
library.addBook(Book('Geometry', 'Jeff Potter', '0596805888', 22))
library.addBook(Book('Math', 'George Harr', '0594805888', 15))
library.addBook(Book('English', 'James Odd', '0596225888', 10))
library.addBook(Book('Physics', 'Jeff Potter', '0597884512', 18))
print(library.searchBookISBN('0594805888'))
print(library.searchBookAuthor('George Harr'))
print(library.searchUnderPrice(20))

Source Link

answered Nov 24, 2017 at 11:19

Peilonrayz ♦

44.5k
7
80
158

Your design doesn't follow best practice. Your Library class should create a library when you use __init__, rather than a book. And so I'd split your code in half, create two classes; one for books, the other for the library.
You shouldn't mutate data on Libary. This is as it mutates it on the class, rather than on the instance. And so if you make a new instance, then it will be tied to all the other instances.
When making the book class. This can significantly be simplified with collections.namedtuple. Such as:
```
 Book = namedtuple('Book', 'name author ISBN price')
```

And so I'd change your code to:

from collections import namedtuple


class Library(object):
    def __init__(self):
        self.books = []

    def addBook(self, book):
        self.books.append(book)

    def searchBookISBN(self, ISBN):
        for book in self.books:
            if book.ISBN == ISBN:
                return book

    def searchBookAuthor(self, author):
        written_by_author
        for book in self.books:
            if book.author == author:
                written_by_author.append(book)
        return written_by_author

    def searchUnderPrice(self, price):
        books_under_price = []
        for book in self.books:
            if book.price < price:
                books_under_price.append(book)
        return books_under_price


Book = namedtuple('Book', 'name author ISBN price')

library = Library()
library.addBook(Book('Geometry', 'Jeff Potter', '0596805888', 22))
library.addBook(Book('Math', 'George Harr', '0594805888', 15))
library.addBook(Book('English', 'James Odd', '0596225888', 10))
library.addBook(Book('Physics', 'Jeff Potter', '0597884512', 18))
print(library.searchBookISBN('0594805888'))
print(library.searchBookAuthor('George Harr'))
print(library.searchUnderPrice(20))

After this, I'd recommend:

You read and follow PEP 8, so that your code is easier to read.
You learn list comprehensions

Which would further improve your code to:

class Library(object):
    def __init__(self):
        self.books = []

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

    def book_with_ISBN(self, ISBN):
        for book in self.books:
            if book.ISBN == ISBN:
                return book

    def books_by_author(self, author):
        return [book for book in self.books if book.author == author]

    def books_under_price(self, price):
        return [book for book in self.books if book.price < price]