[PATCH] smartset: add a "toset" method

[Augie Fackler]

> On Jun 3, 2017, at 4:58 PM, Jun Wu <quark at fb.com> wrote:
> 
> Excerpts from Jun Wu's message of 2017-06-03 13:54:18 -0700:
>> Excerpts from Augie Fackler's message of 2017-06-03 16:25:52 -0400:
>>> I like where you're headed with this, but I think I'd rather have
>>> classes with an efficient way to produce a set implement toset
>>> themselves and leave the default implementation as set(self) so we can
>>> avoid the spooky action at a distance implied by this hasattr().
>> 
>> The problem is the lack of ability to customize "set(smartset)" logic in the
>> Python language. "set" will always treat "smartset" as an iterable, which
>> makes the operation O(N log N) even if we have O(1) fast path.
>> 
>> Between syntactic simplicity and ability to have fast code path, I want to
>> choose the latter.
>> 
>> For "hasattr", do you mean we might need to do hasattr(x, 'toset')? That
>> looks like a sign of poorly designed APIs (sometimes return a smartset,
>> sometimes return a set, so you never know what "x" is). It shouldn't happen
>> in a clean codebase.
> 
> Sorry. I misunderstood the whole paragraph. I'll move "toset" from
> "abstractsmartset" to subclasses.

You should probably still have a toset() on abstractsmartset if you intend for it to be a mandatory part of the interface (which I recommend, per Liskov), but have it either do set(self) or (probably better) raise NotImplementedError.

> 
> Good advice by the way!

Thanks! Glad you like it. :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170603/e7dfbadd/attachment.html>