[Tutor] associating two objects without ORM and processing a text file

[neubyr]

On Mon, Feb 11, 2013 at 7:34 PM, Steven D'Aprano <steve at pearwood.info>wrote:

> On 12/02/13 10:16, Alan Gauld wrote:
>
>> On 11/02/13 22:49, neubyr wrote:
>>
>>  is right approach to implement 'list_by_author' function as a class
>>> method is typically used as an alternative constructor.
>>>
>>
>> Not at all that is only one use case for class methods.
>> A class method is *any* method that operates on the whole class of
>> objects - i.e. all instances(potentially including those still to be
>> created)
>>
>
>
> Strictly speaking, a class method is just a method which takes as its
> first argument the class itself, not the instance. What it does with
> that is completely open.
>
> The usual thing is to use class methods for alternative constructors,
> e.g. Decimal.fromfloat.
>
> If the class keeps a list of all instances, then the class method
> could walk the list and operate on each instance in turn. (But why
> would you do that?)
>
> If the class method modifies the class itself, then it could indirectly
> have an effect on each instance.
>
> Although class methods could do anything, it is hard to think of
> actually useful things for them to do apart from being used as a
> constructor.
>
> [...]
>
>  Here I am
>>> returning list of objects and not just an object.
>>>
>>
>> Which is to say a subset of the class Book.
>> Therefore quite reasonably a class method.
>>
>
> Just a minute, I think that is completely wrong. A Book is not a set,
> so how can you have subset of it?
>
> What is a subset of "Pride and Prejudice"? Perhaps chapter 5.
>
> There are more problems with this idea that you query the Book to get
> a list of books by some author. Suppose you did this:
>
> prpr = Book("Pride and Prejudice", "Jane Austin")
> prpr.list_by_author()
>
> Now *each and every* book is responsible for tracking all the other
> books by the same author. This is a lousy design. Even worse:
>
> prpr.list_by_author("Jane Austin")
>
>
> since now books are responsible for tracking ALL books by ALL authors,
> since the caller could say:
>
> prpr.list_by_author("Leo Tolstoy")
>
>
> Of course, books should know their own author, not the authors of other
> books, but authors should know all their own books:
>
> author = prpr.author  # --> Author("Jane Austin")
>
> author.get_books()  # --> return a list of books by this author
>
>
> This is an argument for making Authors a class, with behaviour, rather
> than just a string.
>
>
>
>  @classmethod
>>> def list_by_author(self,author):
>>>     """ Return list of books of an author """
>>>     bookfile = config.bookfile
>>>     books = [] # empty list - used as list of Books
>>>
>> [snip]
>
>
> First off, by convention the first argument to a class method should be
> called "cls", not "self".
>
> Secondly, here you are relying on a mysterious global "config", which
> points to a bookfile. What does this have to do with a book?
>
> - Does a nail keep track of the packet it came from?
>
> - Why should a book keep track of the catalog it was listed in?
>
> This should be a top level function, not a Book method.
>
> The rest of the method's design is also poor. You have already read
> the file once, to get the initial set of books. So why read the file
> again, every time you want to get some piece of information.
>
> Big databases, capable of holding billions of pieces of data, have
> to use disk-based storage because you can't keep that much data in
> memory at once. For anything smaller, you should only read and write
> to disk for persistence, everything else should use in-memory data
> structures. In this case, that means a dict.
>
>
>
>  return books # return list of books
>>>
>>
> Really? Thank goodness for the comment, I wouldn't have understood
> that line of code otherwise!
>
> *wink*
>
>
>
:)

Thank you for your inputs Steven. I will keep your suggestions in mind
while refactoring this code.

I am not following your comment on opening books file twice in
list_by_author method. I have opened it only once and then reading each
line while checking for a regex match. Am I missing something?

- N
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/tutor/attachments/20130212/4eee8e02/attachment.html>