Code suggestions?

MRAB python at mrabarnett.plus.com
Sat Sep 21 15:50:08 EDT 2013


On 21/09/2013 15:00, Benjamin Schollnick wrote:
> Folks,
>
> I am re-writing an image gallery application that I wrote in Twisted.
>
> As part of this rewrite, I am attempting to simplify the process of
> managing the directory listings.
>
> Would anyone have any suggestions on streamlining or optimizing this code?
>
> The raw version of this file, is available from dropbox.
>
> https://dl.dropboxusercontent.com/u/241415/unified.py
>
> If you are testing with RAR files, you'll need rarfile.py
>
> https://dl.dropboxusercontent.com/u/241415/rarfile.py
>
> If anyone knows a better built-in Python 2.66/2.75 solution, that would
> be helpful.
>
> I eventually expect to support caching, but I think that will be an
> application level feature, and not built-in to this module.
>
> The entire idea is to unify (thus unified.py) behavior better files,
> directories, and archives.
>
> Yes, there is differences in the data coming back, but the calling
> logic, is identical.
>
> So far, this rewrite has tremendously simplified the code for the image
> gallery.  If there is any interest, I'm willing to github unified or the
> image gallery.
>
[snip]
Some comments:-

Using "import *" is generally regarded as a Bad Thing.

Class names usually don't contain underscores, therefore
"UnifiedDirectory" would be better.

It's better to create file paths using os.path.join(...) instead of
"manually" using "+".

When checking for None, use "is" and "is not", not "==" and "!=".

It's almost NEVER a good idea to use a bare except. Specify which
exception you're trying to catch.

The function "populate_file_data_from_filesystem" can return True,
False (printing a message just before it does so, which makes it harder
to use as part of a library), or None (because there's a bare return).
It would be better if it raised an exception in the case of an error,
returning None otherwise (a bare return).




More information about the Python-list mailing list