Changing the subject line for follow up.

On Wed, Dec 29, 2021 at 04:28:18PM +0100, Levente Uzonyi wrote:
> 
> On Mon, 27 Dec 2021, Thiede, Christoph wrote:
> >
> >two goals: First of all, I was assuming that we aim to provide a 
> >comprehensive set of features in the Collections package, and having no 
> >possibility to customize the key comparison in weak key dictionaries 
> >sounded like a
> >logical gap to me. There are no senders of?WeakKeyDictionary in the Trunk 
> >but it IMO?still adds value to the package.?Second, I am actually having a 
> >use?case for?PluggableWeakKeyDictionary in a third-party package
> >(SimulationStudio#34) where I need to compare keys by mirror primitives 
> >but still need to hold them weakly.
> >
> >
> >I have uploaded this class directly to Trunk because I could not see any 
> >harm in it and did not want to create another helper package for 
> >Collections. If there was anything wrong with this approach, I apologize 
> >and we can
> >remove this class immediately again. Still, I would find 
> >a?PluggableWeakKeyDictionary in the Trunk useful.
> >
> >
> >>?And I think there might be a better?way to do what you want to achieve 
> >than copying all those methods.
> >
> >
> >What approach were you having in mind concretely? :-)
> 
> It is very tempting to make all dictionaries pluggable because it makes 
> them a lot more flexible at the cost of a little overhead (both memory 
> usage and computation time).
> 
> There are probably cases where the "classic" Dictionary and 
> IdentityDictionary duo needs to be small (no memory overhead) and fast (no 
> run-time overhead), so making those pluggable (in case of Dictionary) or 
> non-existant (in case of IdentityDictionary) would not pass with the 
> community.
> 

The idea of making all dictionaries pluggable is appealing because
it would simplify the Dictionary hierarchy. But it is not simple to
do, because we cannot easily add instance variables to the special
class MethodDictionary, which inherits from Dictionary.

One way to do handle this would be to let Dictionary and MethodDictionary
inherit from a common abstract dictionary class. I tried it out and
it seems to work well (although I do have a few failing tests that I
did not bother to follow up on). The attached screen shot shows the
hierarchy after refactoring and moving pluggable-ness up to class
Dictionary.

Note that I am not suggesting this for trunk, just offering it up a
possible approach.

> But making other dictionaries (and Sets of course) pluggable should just 
> work due to their marginal use (except for special classes like 
> MethodDictionary).
> 
> So, instead of introducing a mix of the two classes, I suggest you 
> should consider simply changing the superclass of WeakKeyDictionary to
> PluggableDictionary.
> There may be unwanted side-effects that need to be reviewed and fixed, but 
> that's not very likely.
> Anyway, I suggest you give this a try and see how it works for your 
> project. If it looks right, push it to the Trunk (or the Inbox if you're 
> not 100% sure it's ready for general consumption).
> 

This seems like very good advice to me.

Dave

-------------- next part --------------
A non-text attachment was scrubbed...
Name: Collections-Unordered-AbstractDictionary.png
Type: image/png
Size: 69472 bytes
Desc: not available
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211230/d85f7d3a/attachment-0001.png>