On Thu, Mar 25, 2010 at 3:52 PM, Ingy dot Net <[email protected]> wrote:
> On Wed, Mar 24, 2010 at 3:06 PM, Gary Bernhardt <[email protected]>
> wrote:
>>
>> On Wed, Mar 24, 2010 at 2:42 PM, Ingy dot Net <[email protected]> wrote:
>> > Thanks fellas,
>> >
>> > Those are fine ideas, but I guess what I was really hoping for was not
>> > so
>> > much help with that specific question, but some review of my newbie
>> > python
>> > module code.
>> >
>> > In many ways, Python is very straightforward, but in other aspects I
>> > spent
>> > hours looking for the right way to do something. Often times I wasn't
>> > sure I
>> > got it right. I did read PEP-0008 and an old OSCON talk called "Code
>> > Like a
>> > Pythonista". Still it would be great to have some old hands give it a
>> > quick
>> > look. It's not that much code.
>
> Hi Gary,
>
> Thanks for doing this. It was just the kind of thing I was hoping for. :)
>
> I just pushed out http://pypi.python.org/pypi/pyplay/0.5
>
>> Here are some small-scale things that stand out in a skim, in order as
>> I read through the file:
>>
>> - There are no tests for this code! ;)
>
> Fixed! But see bottom of msg...
>
>> - Don't call modules.__reversed__; magic methods are magic for a
>> reason and should very rarely be called directly. You want the
>> reversed() built-in instead.
>
>
> Fixed!
>
>> - "del"ing local variables at the end of a function doesn't do
>> anything; it just destroyed the local reference, which would've been
>> destroyed when the function returned anyway.
>
> Actually it does do something in this case. When I ran the command:
>
> >>> y(globals())
>
> inside pyplay, it showed the 'command' variable, which is an internal detail
> that the user doesn't need to know about. I get your point in general
> though.
>
>> - Short-circuiting "and"/"or" tricks are dangerous – in Python, there
>> many false values (0, [], {}, "", etc.) that can trip you up. Instead,
>> use a proper if statement or the ternary syntax: "val1 if condition
>> else val2".
>
> Fixed! I left in
>
> dir = ENV_CONFIG_DIR or LOCAL_CONFIG_DIR or HOME_CONFIG_DIR
>
> but I just set those variables, so probably no surprises.
>
> I'm glad to know about this ternary syntax. I didn't know it existed. I
> assumed it didn't because the familiar ?/: operators were not there. Pythons
> ternary syntax reads nicely.
Guido opposed a ternary operator for many years. He prefers multiline
'if', and others used the "and or" syntax:
x = CONDITION and TRUE_VALUE or FALSE_VALUE
The problem is that this is usually but not always equivalent to
Perl's "? :" operator. It produces the opposite result if B is false
(you'd get C instead of B). Often this is not a problem because B is a
constant, but it kept tripping people up.
The clamor for a real ternary operator would never die, so finally
Guido relented for Python 2.5. But the community couldn't agree on a
syntax. There was equal support for "? :", "if CONDITION then
TRUE_VALUE else FALSE_VALUE", and several others.
So Guido decided to turn the 'if' construct around to:
x = TRUE_VALUE if CONDITION else FALSE_VALUE
The reason is that it's rare for both branches to be equally
important. Normally there's a "common" case and an "unusual" case.
This syntax allows the common case to be the most prominent, rather
than the condition.
> I'll have to see how it looks when nested.
I wouldn't nest them. The ternary operator is tolerated only because
it's more readable with short expressions. But nesting them is pretty
*un*readable.
>> - Instead of asking a dictionary for a key and catching the KeyError
>> (four lines), consider my_dict.get(key, default_value) (one line).
Also note ``x = my_dict.pop(key, default)`` to get and delete the item
in one step. This useful to extract a few known items.
def foo(a, b=None, **kw):
# There are a few optional args I don't want to make
# positional for some reason.
red = kw.pop("red", None)
blue = kw.pop("blue", None)
if kw:
keys = ", ".join(sorted(kw))
# Hint: iterating a dict yields the keys.
raise TypeError("illegal keyword args: %s" % keys)
``dict.setdefault()`` and ``collections.defaultdict`` (starting in
Python 2.6) are also pretty useful.
--
Mike Orr <[email protected]>
