[Freeipa-devel] [PATCHES] 0696-0710 More modernization

[Christian Heimes]

On 2015-08-24 17:31, Petr Viktorin wrote:
>>>> 0701.2-Use-Python3-compatible-dict-method-names
>>>> NACK
>>>> Why are you replacing iteritems() with items() instead of using
>>>> six.iteritems()?
> 
> It looks cleaner, and it will be easier to clean up after six is dropped.
> Also, the performance difference is negligible if the whole thing is
> iterated over. (On small dicts, which are the majority of what iteritems
> was used on, items() is actually a bit faster on my machine.)

Right, for small dicts the speed difference is negligible and favors the
items() over iteritems(). For medium sized and large dicts the iterators
are faster and consume less memory.

I'm preferring iterator methods everywhere because I don't have to worry
about dict sizes.

>>>> 0710.2-Modernize-use-of-range
>>>> NACK
>>>> Please use six.moves.range. It defaults to xrange() in Python 2.
> 
> Why? What is the benefit of xrange in these situations?
> 
> Like with iteritems in 0701, when iterating over the whole thing, the
> performance difference is negligible. I don't think a few microseconds
> outside of tight loops are worth the verbosity.

It's the same reasoning as in 0701. As long as you have a small range,
it doesn't make a difference. For large ranges the additional memory
usage can be problematic.

In all above cases the iterator methods and generator functions are a
safer choice. A malicious user can abuse the non-iterative methods for
DoS attacks. As long as the input can't be controlled by a user and the
range/dict/set/list is small, the non-iterative methods are fine. You
have to verify every location, though.

I'm usually too busy with other stuff (aka lazy) to verify these
preconditions...

Christian


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150825/d7c5c896/attachment.sig>