Code review comment for lp://staging/~thumper/launchpad/scanner-awesomeness

Revision history for this message
Andrew Bennetts (spiv) wrote :

Michael Hudson wrote:
> Review: Approve
> I pointed out a couple of typos on IRC.
>
> I think a test of CachedIterator that iterated two iterators in parallel like so:
>
> ci = CachedIterator([...])
> i1 = iter(ci)
> assert i1.next() == ...
> i2 = iter(ci)
> assert i2.next() == ...
> assert i1.next() == ...
>
> I think it'll pass fine, but it's something my devious mind wants to see tested.

You need to be more devious than that: the assumption CachedIterator is making
is that its __iter__ will never call itself. You'd think that's a reasonable
assumption, but I've seen code that accidentally does it. The more interacting
layers of code you have, with lazy DB iterators and so forth, the more likely it
is...

And boy is it confusing as hell when it does happen and you get “impossible”
results from apparently simple code.

So, try this:

class EvilIterator(object):
    def __init__(self):
        self.elem_source = iter('abcdef')
    def next(self):
        e = self.elem_source.next()
        if e == 'c':
            print 'evil:', list(ci)
        return e

ci = CachingIterator(EvilIterator())
i1 = iter(ci)
i2 = iter(ci)
for elem in i1:
    print '1:', elem
for elem in i2:
    print '2:', elem

-Andrew.

« Back to merge proposal