[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