On Mon, Feb 11, 2013 at 7:34 PM, Steven D'Aprano <st...@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
_______________________________________________ Tutor maillist - Tutor@python.org To unsubscribe or change subscription options: http://mail.python.org/mailman/listinfo/tutor