On 2010-10-12, Jonas H. <jo...@lophus.org> wrote:
> Just a few pointers, looks quite good to me for a newbie :)
Thanks!
> * Less action in __init__.
I'm a bit curious about this. The __init__ functions in this are, at
least for now, pretty much doing only what's needed to create the objects
from their inputs.
> * Use `open` instead of `file` to open a file
Done.
> * Have a look at context managers for file handling (avoids doing
> error-prune stuff like __del__)
Okay. I wasn't at all sure the __del__ was needed.
> * Your `del` in line 464 is useless. A reference will be removed from
> the object bound to the local variable 'source' anyway because of the
> re-assignment.
Oh. D'oh. You can see the C programmer instinct there; that was the
sort of thing that would, in C, be:
for (i = 0; i < n; ++i)
free(array[i]);
But of course, that doesn't work. I guess that leads to a general question:
Is it safe for me to assume that all my files will have been flushed and
closed? I'd normally assume this, but I seem to recall that not every
language makes those guarantees.
> * according to common Python style guides you should not use underscores
> in class names.
Makes sense. I was finding "CArgument" hard to read, and couldn't think
of a better name.
> * no need for 'r' in `open` calls ('r' is the default mode)
'k.
> * `line == ''` can be written more pythonic as `not line`
'k.
> * `str.{r,l,}strip` removes '\n\t\r ' by default, no need for an
> argument here (line 440 for example)
Ahh, good to know.
> * you might want to pre-compile regular expressions (`re.compile`)
Thought about it, but decided that it was probably more complexity than I
need -- this is not a performance-critical thing. And even if it were, well,
I'm pretty sure it's I/O bound. (And on my netbook, the time to run this
is under .2 seconds in Python, compared to 15 seconds in shell, so...)
> * raising `Exception` rather than a subclass of it is uncommon.
Okay. I did that as a quick fix when, finally having hit one of them,
I found out that 'raise "Error message"' didn't work. :) I'm a bit unsure
as to how to pick the right subclass, though.
-s
--
Copyright 2010, all wrongs reversed. Peter Seebach / usenet-nos...@seebs.net
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
I am not speaking for my employer, although they do rent some of my opinions.
--
http://mail.python.org/mailman/listinfo/python-list
