<div id="__MailbirdStyleContent" style="font-size: 10pt;font-family: Arial;color: #000000;text-align: left" dir="ltr">
Hi Christoph --<div><br></div><div>> Or is a new major release a good chance for a breaking change like this?</div><div><br></div><div>No. A major release should be no excuse for poor design choices that break backwards compatibility without much gain :-) The question you might want to ask yourself is not that simple.</div><div><br></div><div>Best,</div><div>Marcel</div><div class="mb_sig"></div>
<blockquote class="history_container" type="cite" style="border-left-style: solid;border-width: 1px;margin-top: 20px;margin-left: 0px;padding-left: 10px;min-width: 500px">
<p style="color: #AAAAAA; margin-top: 10px;">Am 02.01.2022 23:10:16 schrieb christoph.thiede@student.hpi.uni-potsdam.de <christoph.thiede@student.hpi.uni-potsdam.de>:</p><div style="font-family:Arial,Helvetica,sans-serif">
Hi Levente, hi Dave, hi all,<br>
<br>
HNY! :-)<br>
<br>
> So, instead of introducing a mix of the two classes, I suggest you should consider simply changing the superclass of WeakKeyDictionary to PluggableDictionary.<br>
<br>
This sounds like a good idea. I will address this soon, thank you!<br>
<br>
---<br>
<br>
If we want to think larger:<br>
<br>
> It is very tempting to make all dictionaries pluggable because it makes <br>
> them a lot more flexible at the cost of a little overhead (both memory <br>
> usage and computation time).<br>
> <br>
> There are probably cases where the "classic" Dictionary and <br>
> IdentityDictionary duo needs to be small (no memory overhead) and fast (no <br>
> run-time overhead), so making those pluggable (in case of Dictionary) or <br>
> non-existant (in case of IdentityDictionary) would not pass with the <br>
> community.<br>
<br>
> The idea of making all dictionaries pluggable is appealing because<br>
> it would simplify the Dictionary hierarchy. But it is not simple to<br>
> do, because we cannot easily add instance variables to the special<br>
> class MethodDictionary, which inherits from Dictionary.<br>
<br>
The idea is appealing indeed. :-)<br>
<br>
But I that believe another instvar on Dictionary would indeed bother some people, thinking of this amazing debate one year ago: http://forum.world.st/The-Inbox-Collections-cmm-874-mcz-td5110652.html<br>
<br>
So I fear that pluggability needs indeed to be opt-in. Or is a new major release a good chance for a breaking change like this?<br>
<br>
Best,<br>
Christoph<br>
<br>
<span style="color: #808080">---<br>
</span><span style="color: #808080"><i>Sent from </i></span><span style="color: #808080"><i><a href="https://github.com/hpi-swa-lab/squeak-inbox-talk"><u><span style="color: #808080">Squeak Inbox Talk</span></u></a></i></span><br>
<br>
On 2021-12-30T17:54:19-05:00, lewis@mail.msen.com wrote:<br>
<br>
> Changing the subject line for follow up.<br>
> <br>
> On Wed, Dec 29, 2021 at 04:28:18PM +0100, Levente Uzonyi wrote:<br>
> > <br>
> > On Mon, 27 Dec 2021, Thiede, Christoph wrote:<br>
> > ><br>
> > >two goals: First of all, I was assuming that we aim to provide a <br>
> > >comprehensive set of features in the Collections package, and having no <br>
> > >possibility to customize the key comparison in weak key dictionaries <br>
> > >sounded like a<br>
> > >logical gap to me. There are no senders of?WeakKeyDictionary in the Trunk <br>
> > >but it IMO?still adds value to the package.?Second, I am actually having a <br>
> > >use?case for?PluggableWeakKeyDictionary in a third-party package<br>
> > >(SimulationStudio#34) where I need to compare keys by mirror primitives <br>
> > >but still need to hold them weakly.<br>
> > ><br>
> > ><br>
> > >I have uploaded this class directly to Trunk because I could not see any <br>
> > >harm in it and did not want to create another helper package for <br>
> > >Collections. If there was anything wrong with this approach, I apologize <br>
> > >and we can<br>
> > >remove this class immediately again. Still, I would find <br>
> > >a?PluggableWeakKeyDictionary in the Trunk useful.<br>
> > ><br>
> > ><br>
> > >>?And I think there might be a better?way to do what you want to achieve <br>
> > >than copying all those methods.<br>
> > ><br>
> > ><br>
> > >What approach were you having in mind concretely? :-)<br>
> > <br>
> > It is very tempting to make all dictionaries pluggable because it makes <br>
> > them a lot more flexible at the cost of a little overhead (both memory <br>
> > usage and computation time).<br>
> > <br>
> > There are probably cases where the "classic" Dictionary and <br>
> > IdentityDictionary duo needs to be small (no memory overhead) and fast (no <br>
> > run-time overhead), so making those pluggable (in case of Dictionary) or <br>
> > non-existant (in case of IdentityDictionary) would not pass with the <br>
> > community.<br>
> > <br>
> <br>
> The idea of making all dictionaries pluggable is appealing because<br>
> it would simplify the Dictionary hierarchy. But it is not simple to<br>
> do, because we cannot easily add instance variables to the special<br>
> class MethodDictionary, which inherits from Dictionary.<br>
> <br>
> One way to do handle this would be to let Dictionary and MethodDictionary<br>
> inherit from a common abstract dictionary class. I tried it out and<br>
> it seems to work well (although I do have a few failing tests that I<br>
> did not bother to follow up on). The attached screen shot shows the<br>
> hierarchy after refactoring and moving pluggable-ness up to class<br>
> Dictionary.<br>
> <br>
> Note that I am not suggesting this for trunk, just offering it up a<br>
> possible approach.<br>
> <br>
> > But making other dictionaries (and Sets of course) pluggable should just <br>
> > work due to their marginal use (except for special classes like <br>
> > MethodDictionary).<br>
> > <br>
> > So, instead of introducing a mix of the two classes, I suggest you <br>
> > should consider simply changing the superclass of WeakKeyDictionary to<br>
> > PluggableDictionary.<br>
> > There may be unwanted side-effects that need to be reviewed and fixed, but <br>
> > that's not very likely.<br>
> > Anyway, I suggest you give this a try and see how it works for your <br>
> > project. If it looks right, push it to the Trunk (or the Inbox if you're <br>
> > not 100% sure it's ready for general consumption).<br>
> > <br>
> <br>
> This seems like very good advice to me.<br>
> <br>
> Dave<br>
> <br>
> -------------- next part --------------<br>
> A non-text attachment was scrubbed...<br>
> Name: Collections-Unordered-AbstractDictionary.png<br>
> Type: image/png<br>
> Size: 69472 bytes<br>
> Desc: not available<br>
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211230/d85f7d3a/attachment-0001.png><br>
> <br>
>
</div></blockquote></div>