[PATCH] mq: refactor usage of repo.branchmap().iteritems() with itervalues()

Kevin Bullock kbullock+mercurial at ringworld.org
Thu Sep 26 13:29:57 CDT 2013


On 26 Sep 2013, at 12:28 PM, Brodie Rao wrote:

> On Thu, Sep 26, 2013 at 6:57 AM, Augie Fackler <raf at durin42.com> wrote:
>> 
>> Agreed. At /least/ parenthesize the nested generator expression. Or
>> something. This is mq, perf isn't exactly critical.
> 
> There's no nested generator expression (or nested anything). This is a
> single list comprehension that has two loops. The list is built from
> the results of the inner loop. If you're not familiar with this
> syntax, you can read more about it here:
> http://docs.python.org/2/tutorial/datastructures.html#list-comprehensions
> 
> We do this elsewhere in the code base. There's no parentheses that you
> could add to this without introducing syntax errors.

Be that as it may I find the double-`for` syntax bloody unreadable. If the more readable form doesn't perform significantly differently, I'd much rather we use that.

<aside>
The problem is that order of the `for`s is precisely and deliberately backwards: by using a normal, simple list comprehension, you turn

    
    squares = []
    for x in range(10):
        squares.append(x**2)

into

    squares = [x**2 for x in range(10)]

...thus _reversing_ the order of the operation on each element and the iteration to get the elements. But the double-`for` case, by failing to _fully_ reverse the syntactical order, separates the inner iteration from the operation it's performing by throwing the outer iteration in the middle. It's totally nonsensical.
</aside>

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock



More information about the Mercurial-devel mailing list