On Thu, Mar 26, 2015 at 6:21 AM, Daniel Gruno <humbed...@apache.org> wrote:
>
>
> On 2015-03-26 12:16, Greg Stein wrote:
>
>> On Thu, Mar 26, 2015 at 4:17 AM, <humbed...@apache.org> wrote:
>>
>>> ...
>>> +++ steve/trunk/pysteve/lib/plugins/fic.py Thu Mar 26 09:17:56 2015
>>>
>>> ...
>>> @@ -65,15 +65,17 @@ def tallyFIC(votes, issue):
>>> matrix[letter] += numseats - i
>>> i += 1
>>>
>>> +
>>> # Start counting
>>> - winners = [l for l in matrix if matrix[l] in
>>> heapq.nlargest(numseats,
>>> matrix.values())]
>>> + sorted_matrix = sorted(matrix.items(), key=operator.itemgetter(1))
>>> + winners = [l[0] for l in sorted_matrix if matrix[l[0]] in
>>> heapq.nlargest(numseats, matrix.values())]
>>>
>>> # Compile list of winner names
>>> winnernames = []
>>> x = 0
>>> for c in winners:
>>> i = ord(c) - ord('a')
>>> - winnernames.append("%s ( %u)" % ( candidates[i],
>>> heapq.nlargest(numseats, matrix.values())[x]))
>>> + winnernames.append("%s (%u points)" % ( candidates[i],
>>> heapq.nlargest(numseats, matrix.values())[x]))
>>>
>>> I realize that we aren't dealing with time sensitive code, and the data
>> sets are not large ... but the above irks me. The heapq.nlargest() is
>> performed on each iteration of the list comprehension, and on each
>> iteration of the for-loop. Yet the result appears to be constant across
>> all
>> these loops. ?
>>
> Yeah, thanks for spotting that - t'was a silly mistake of shortening the
> code, thereby actually making more work for the interpreter.
> I've committed a change where nlargest is called just once now.
>
> Again, thanks for the review :)
No problem. But don't thank me yet... my next one isn't nice :-P
Cheers,
-g
