Author exarkun
Date 2006-10-04.21:13:34
SpamBayes Score
Marked as misclassified
Logged In: YES 

Some review comments:

  * poplib_as_list should have a class docstring explaining
its purpose.  The name is fairly suggestive, but some prose
would be better.

  * Why duplicate the arguments to POP3_SSL and POP3 in
poplib_as_list.__init__?  Wouldn't it be better if it took
an already-constructed POP3 or POP3_SSL instance?

  * Does subclassing list really buy much here?  None of the
overridden methods upcall, and many list methods aren't
implemented here at all: what if someone calls pop on a
poplib_as_list instance?  Ditto for a bunch of other
operations that one might expect to work on a list.

  * __getslice__ is buggy - the return statement is indented
too far.  __getitem__ and __delitem__ will also pass a slice
instance on the underlying pop method when extended slices
are used.

  * segue from the previous comment - unit tests?  Python
has pretty low overall coverage, but all new code at least
can benefit from being committed initially with a
comprehensive test suite.

  * The docstrings on all of the methods that have them are
pretty good, although the __getattribute__ docstring is more
of an implementation note.

  * Should the library reference be updated as well?

Overall, I'm not sure how useful this feature is.  A generic
wrapper class would probably be beneficial to more people.
Date User Action Args
2007-08-23 16:12:15adminlinkissue1567948 messages
2007-08-23 16:12:15admincreate