Question about TableTraverser __next__ method

[imnisen]

Hi, in TableTraverser code as below , when pop an item from rightBuckets, should it pop from the let side from the rightBuckets list ? Because it is nearer, however pop() function pop from right side.

class TableTraverser(object):
    def __init__(self, table, startNode):
        index = table.getBucketFor(startNode)
        table.buckets[index].touchLastUpdated()
        self.currentNodes = table.buckets[index].getNodes()
        self.leftBuckets = table.buckets[:index]
        self.rightBuckets = table.buckets[(index + 1):]
        self.left = True

    def __iter__(self):
        return self

    def __next__(self):
        """
        Pop an item from the left subtree, then right, then left, etc.
        """
        if len(self.currentNodes) > 0:
            return self.currentNodes.pop()

        if self.left and len(self.leftBuckets) > 0:
            self.currentNodes = self.leftBuckets.pop().getNodes()
            self.left = False
            return next(self)

        if len(self.rightBuckets) > 0:
            self.currentNodes = self.rightBuckets.pop().getNodes()      # <----  Here!
            self.left = True
            return next(self)

        raise StopIteration

Thanks!

[bmuller]

Yes, I think you're right! If you've got time - can you write a test showing the failure? And put that in a pull request along with changing that line to self.rightBuckets.pop(0).getNodes()? Thanks!

[imnisen]

Hi, when I writing test case for TableTraverser , I found it a little hard to make it "unit", as it relies on RoutingTable in its init method. And the buckets of routing table is generated auto by flush() method, with fixed kbucket range from 0 to 2**160. So there are three ways (more like design choice ) I considering make this test case.

First, I can make nodes id calculated accurately , so that when call addContact to routing table, it can be insert precisely into different buckets(maybe bucket will split). After that I can check if the TableTraverser iterated in right order I want. The shortcoming is calculating lots of node ids which will cause the bucket split and insert right is painful, also make the test case hard to understand. Last, test case rely on addContact.

Second way is what I made in the pull request, I make fake buckets and nodes, then replace routing table's default range [0,2**160] bucket with my fake ones. Then test the TableTraverser. The shortcoming is the test case relies on RoutingTable and it's methods, which make it not "unit" enough.

The third way is make TableTraverser instance my own, and replace it's currentNodes, leftBuckets,rightBuckets fields with my fake ones. So I can test TableTraverser iteration independently.However, when make TableTraverser instance, I also need a fake empty routing table to call it __init__ method. After instance it, then change its value, seems a little cumbersome.

Looking forward to your advice. Thanks!

[bmuller]

Hey @imnisen - thanks for the PR! The way you wrote the test was fine. In general, since your'e testing TableTraverser it's fine to rely on RoutingTable and its methods, because ideally RoutingTable would be entirely tested separately in its own unit tests and could therefore be relied on. Thanks for the help! I'll make a minor release for this fix.