[squeak-dev] The Trunk: Collections-ct.972.mczThiede, Christoph Christoph.Thiede at student.hpi.uni-potsdam.deWed Dec 29 02:49:01 UTC 2021
Thank you for the very helpful critique, Dave! I have removed PluggableWeakKeyDictionary from the Collections package again. When I come up with another idea of this extent, I will upload it to the inbox only, I promise. :-) For further potential extensions to Collections and other packages, we should maybe document an answer to these questions: Do protocols such as the Collection hierarchy function as an internal support package for other Trunk packages only, or do they also address external client packages? Until today, I have always assumed the latter and thought of the Collections package as a public API. Is a protocol only useful and legitimate if it is used in the Trunk? As an example, WeakKeyDictionary has no senders in the Trunk, too, but I see a two-digit number<https://sourcegraph.com/search?q=context:global+WeakKeyDictionary+lang:Smalltalk+-lang:c%23&patternType=literal> of dependent packages only on GitHub and most of these users appear legitimate to me. Other examples include resumable test failures, traits, and promises - to me, all of them appear to be valid and useful concepts. They may appear as dead concepts in a plain Trunk image, but there are enough external packages and projects making real use of them. Maintenance and discoverability are nevertheless two very important aspects, of course. Personally, I can still think of valid use cases for a PluggableWeakKeyDictionary (at least outside of the Trunk - see my previous post for details), and I am open to better implementation strategies. But this decision depends on the majority of voices. I'm looking forward to an interesting discussion. :-) Best, Christoph ________________________________ Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von David T. Lewis <lewis at mail.msen.com> Gesendet: Mittwoch, 29. Dezember 2021 00:11:49 An: The general-purpose Squeak developers list Betreff: Re: [squeak-dev] The Trunk: Collections-ct.972.mcz I would advise against including PluggableWeakKeyDictionary in trunk at the present time. It can be added later if the need arises. Why? 1) Cognitive clutter. Adding things that are not actually necessary means that the reader has more to understand when looking at the Dictionary hierarchy. 2) Refactorability. If in the future someone wants to make pluggable-ness more general for some reason, then having it present in two places in the Dictionary hierarchy makes the job more difficult. 3) Housekeeping. It is easy to add something to the image, but very difficult to make it go away. Imagine that someone five years from now wants to deprecate PluggableWeakKeyDictionary, perhaps just because it is not used, or maybe because they have a better refactoring in mind. Once it is in trunk, you can never be certain that it is not referenced by some external package, so things like this tend to stay around for a long time even if they serve no purpose. Dave On Mon, Dec 27, 2021 at 07:36:16PM +0000, Thiede, Christoph wrote: > Hi Levente, > > > 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<https://github.com/LinqLover/SimulationStudio/pull/34/files/3137fe3772b288654d377fc9470302ab2fd26148>) 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? :-) > > > Best, > > Christoph > > ________________________________ > Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Levente Uzonyi <leves at caesar.elte.hu> > Gesendet: Montag, 27. Dezember 2021 18:35:12 > An: squeak-dev at lists.squeakfoundation.org > Cc: packages at lists.squeakfoundation.org > Betreff: Re: [squeak-dev] The Trunk: Collections-ct.972.mcz > > Hi Christoph, > > What is your goal with such dictionary? > It's now in the Trunk with no users. And I think there might be a better > way to do what you want to achieve than copying all those methods. > > > Levente > > On Mon, 27 Dec 2021, commits at source.squeak.org wrote: > > > Christoph Thiede uploaded a new version of Collections to project The Trunk: > > http://source.squeak.org/trunk/Collections-ct.972.mcz > > > > ==================== Summary ==================== > > > > Name: Collections-ct.972 > > Author: ct > > Time: 27 December 2021, 1:19:57.212181 am > > UUID: f740e1ad-4410-0745-b615-2986366b7e75 > > Ancestors: Collections-tonyg.971 > > > > Adds PluggableWeakKeyDictionary, the logical combination of PluggableDictionary and WeakKeyDictionary. All methods are copied literally from PluggableDictionary, but the class inherits from WeakKeyDictionary rather than from Dictionary. > > > > I do not like all the duplication myself but cannot think of a better solution without using composition (slow, breaking design change) or traits (breaking design change). > > > > =============== Diff against Collections-tonyg.971 =============== > > > > Item was added: > > + WeakKeyDictionary subclass: #PluggableWeakKeyDictionary > > + instanceVariableNames: 'hashBlock equalBlock' > > + classVariableNames: '' > > + poolDictionaries: '' > > + category: 'Collections-Weak'! > > + > > + !PluggableWeakKeyDictionary commentStamp: 'ct 12/27/2021 01:07' prior: 0! > > + I combine the features of PluggableDictionary and WeakKeyDictionary, i.e., clients can supply custom blocks for comparing and hasing my keys, and I hold weakly on my keys.! > > > > Item was added: > > + ----- Method: PluggableWeakKeyDictionary class>>integerDictionary (in category 'instance creation') ----- > > + integerDictionary > > + ^ self new hashBlock: [:integer | integer hash \\ 1064164 * 1009]! > > > > Item was added: > > + ----- Method: PluggableWeakKeyDictionary>>= (in category 'comparing') ----- > > + = anObject > > + "Two dictionaries are equal if > > + (a) they are the same 'kind' of thing. > > + (b) they have the same set of keys. > > + (c) for each (common) key, they have the same value" > > + > > + self == anObject ifTrue: [ ^true ]. > > + self species == anObject species ifFalse: [ ^false ]. > > + hashBlock = anObject hashBlock ifFalse: [ ^false ]. > > + equalBlock = anObject equalBlock ifFalse: [ ^false ]. > > + self size = anObject size ifFalse: [ ^false ]. > > + self associationsDo: [ :association | > > + (anObject at: association key ifAbsent: [ ^false ]) = association value > > + ifFalse: [ ^false ] ]. > > + ^true! > > > > Item was added: > > + ----- Method: PluggableWeakKeyDictionary>>collect: (in category 'enumerating') ----- > > + collect: aBlock > > + "Evaluate aBlock with each of my values as the argument. Collect the resulting values into a collection that is like me. Answer with the new collection." > > + > > + | newCollection | > > + newCollection := (self species new: self size) > > + hashBlock: hashBlock; > > + equalBlock: equalBlock; > > + yourself. > > + self associationsDo: [ :each | > > + newCollection at: each key put: (aBlock value: each value) ]. > > + ^newCollection > > + > > + ! > > > > Item was added: > > + ----- Method: PluggableWeakKeyDictionary>>copyEmpty (in category 'copying') ----- > > + copyEmpty > > + > > + ^super copyEmpty > > + hashBlock: hashBlock; > > + equalBlock: equalBlock; > > + yourself! > > > > Item was added: > > + ----- Method: PluggableWeakKeyDictionary>>equalBlock (in category 'accessing') ----- > > + equalBlock > > + "Return the block used for comparing the elements in the receiver." > > + ^equalBlock! > > > > Item was added: > > + ----- Method: PluggableWeakKeyDictionary>>equalBlock: (in category 'accessing') ----- > > + equalBlock: aBlock > > + "Set a new equality block. The block must accept two arguments and return true if the argumets are considered to be equal, false otherwise" > > + equalBlock := aBlock.! > > > > Item was added: > > + ----- Method: PluggableWeakKeyDictionary>>hashBlock (in category 'accessing') ----- > > + hashBlock > > + "Return the block used for hashing the elements in the receiver." > > + ^hashBlock! > > > > Item was added: > > + ----- Method: PluggableWeakKeyDictionary>>hashBlock: (in category 'accessing') ----- > > + hashBlock: aBlock > > + "Set a new hash block. The block must accept one argument and must return the hash value of the given argument." > > + hashBlock := aBlock.! > > > > Item was added: > > + ----- Method: PluggableWeakKeyDictionary>>scanFor: (in category 'private') ----- > > + scanFor: anObject > > + "Scan the key array for the first slot containing either a nil (indicating an empty slot) or an element that matches anObject. Answer the index of that slot or raise an error if no slot is found. This method will be overridden in various subclasses that have different interpretations for matching elements." > > + > > + | index start size | > > + index := start := (hashBlock > > + ifNil: [ anObject hash ] > > + ifNotNil: [ hashBlock value: anObject ]) \\ (size := array size) + 1. > > + [ > > + | element | > > + ((element := array at: index) == nil or: [ > > + equalBlock > > + ifNil: [ element key = anObject ] > > + ifNotNil: [ equalBlock value: element key value: anObject ] ]) > > + ifTrue: [ ^index ]. > > + (index := index \\ size + 1) = start ] whileFalse. > > + self errorNoFreeSpace! > > > > Item was added: > > + ----- Method: PluggableWeakKeyDictionary>>scanForEmptySlotFor: (in category 'private') ----- > > + scanForEmptySlotFor: anObject > > + "Scan the key array for the first slot containing an empty slot (indicated by a nil). Answer the index of that slot. This method will be overridden in various subclasses that have different interpretations for matching elements." > > + > > + | index start size | > > + index := start := (hashBlock > > + ifNil: [ anObject hash ] > > + ifNotNil: [ hashBlock value: anObject ]) \\ (size := array size) + 1. > > + [ > > + (array at: index) ifNil: [ ^index ]. > > + (index := index \\ size + 1) = start ] whileFalse. > > + self errorNoFreeSpace! > > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211229/6c656872/attachment.html>
More information about the Squeak-dev mailing list |