[PATCH] smartset: add a "toset" method

Augie Fackler raf at durin42.com
Sat Jun 3 18:35:53 EDT 2017

> 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>

More information about the Mercurial-devel mailing list