[MERGE] GraphIndex bisection

Robert Collins robertc at robertcollins.net
Tue Oct 9 03:18:43 BST 2007


On Mon, 2007-10-08 at 16:42 +1000, Andrew Bennetts wrote:
> bb:comment
> 
> This isn't a proper review, just a comment on something that jumped
> out at me
> while skimming:
...
> The -1/False/+1/result interface seems a bit distasteful to me.  Partly this is
> because it “result” sounds like it may be any object, but obviously you've
> precluded -1, False and +1 as possible values.  Also partly because mixing
> bools and ints seems likely to cause confusion.  And just generally because
> making one value do triple-duty seems like a kludge.

> Probably just clarifying what 'result' can (or cannot) be would be enough to
> make me happy.  There are four distinct states here, so in that respect this is
> neater than trying to make a two-tuple or something out of it.  But the initial
> impression is of “oh it can be one of a couple of different types legitimately,
> or any other arbitrary object, that seems likely to cause trouble”.

I agree that its a little untasty. I felt it was more tasty that a
tuple: (-1, None) (1, None), (False, None), (True, result).

The docstring is already quite clear about what result can be IMO, I'm
not sure what can be made clearer - clearly it cannot be -1, 1 or False,
but any other value is return to the caller e.g. ('key', None) if you
were to return ((loc, 'key'), None) from the content lookup function.

-Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20071009/9326d228/attachment.pgp 


More information about the bazaar mailing list