[PATCH 17 of 24] churn: sort users with same churn by name

Mads Kiilerich mads at kiilerich.com
Mon Dec 17 19:34:20 CST 2012


Bryan O'Sullivan wrote, On 12/17/2012 10:02 PM:
> On Sun, Dec 16, 2012 at 2:34 PM, Mads Kiilerich <mads at kiilerich.com 
> <mailto:mads at kiilerich.com>> wrote:
>
>     churn: sort users with same churn by name
>
>
> I like the intent of this change.
>
>     This makes the output order well-defined and improves code
>     readability.
>
>     -    sortkey = ((not opts.get('sort')) and (lambda x: -sum(x[1]))
>     or None)
>
>
> Improves? Hardly. That is impenetrable.

Please clarify the intent of your comment. I agree that the removed line 
is non-obvious, but I don't get why you comment on the line I remove and 
don't consider the more explicit implementation and improvement.

It could be ironic, or it could be an opinion that is a consequence of 
too much experience functional programming ...

/Mads

> -    sortkey = ((not opts.get('sort')) and (lambda x: -sum(x[1])) or None)
> -    rate.sort(key=sortkey)
> +    if opts.get('sort'):
> +        rate.sort()
> +    else:
> +        rate.sort(key=lambda x: (-sum(x[1]), x))



More information about the Mercurial-devel mailing list