Launchpad logo and name.


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index ][Thread Index ]

Re: Ready for review: abentley/launchpad/branch-deletion



On 11/02/2008, Tim Penhey <tim@xxxxxxxxxxxxx> wrote:
> On Saturday 09 February 2008 05:59:42 Aaron Bentley wrote:
> > >> +        result = dict((k, ('alter', v)) for k, v in alterations.iteritems())
> > >
> > > Single letter variable names are not generally used.
> > >
> > >        result = dict((associated_object, ('alter', reason))
> > >                      for associated_object, reason in alterations.iteritems())
> > >
> > > While more verbose, I find much easier to read and understand without
> > > having to go to the initial function to work out what was stored.
> >
> > I only use them in list comprehensions and generator expressions.  These
> > forms are not really readable unless they are one or two lines, and
> > longer names tend to blow them out in size.
>
> Well, I find the two line one above much more readable than the one line
> example with single letter variables.
>
> > IME, single-letter variables in are standard practice in generator
> > expressions.  They're used pervasively in PEP289(Generator expresions).
> >    Both PEP8 and Launchpad's PythonStyleGuide are silent on the issue.
> > Other launchpad code, like bugtask.py, also uses them, though not
> > exclusively.
>
> I see that it isn't explicitly mentioned, but that is an oversight.  Single letter
> variables should no longer be used in the launchpad codebase, even if for
> generator expressions.

I was a bit surprised to see Aaron's change to the style guide.

While I do think that one letter identifiers are in almost cases a bad
practice, I agree with Aaron that in list comprehensions and generator
expressions they can sometimes not only be permissible, but even
desirable, when they allow the entire expression to be written on one
line, and when the naming is idiomatic or just obvious.

I'd like to propose that we do make an exception to that rule in such cases.

A few examples for cases where I'd like to use single character identifiers:

 * (do_something(k, v) for k, v in my_dict.items())
   # k, v is idiomatic (i think)

 * (x.attr for x in somethings)
   # the final s in the iterable's name makes it clear what x is.

 * (do_something(c) for c in a_string)
   # the context makes it obvious that x is a character

Tom




This is the launchpad-users mailing list archive — see also the general help for Launchpad.net mailing lists.

(Formatted by MHonArc.)