Forgive me, Eliot, but I feel that is a negative change.  An elegant
class-library design should use as high-level public methods as it
can, and have as _few_ methods as possible that make calls to private,
implementation-specific code (e.g. scanFor:).  Especially in abstract
classes like Dictionary.  I know its not technically abstract, I'm
referring to the fact that it is the superclass for many different
subclasses, including ones in other packages which this change did not
consider -- I'd bet this will break some of them, and the irony will
be that they'll be forced to duplicate the elegance that existed
before in multiple subclasses.  :(

If you are working from a tight loop, you could consider letting the
_style_ of the code advertise its intention to be efficient by calling
the lower-level method(s) directly from there.  If anything, calling the
higher-level one could actually lose that intent on the reader...

It seems this only saves a couple of extra method sends, but for a new
person to understand, because they'll more likely know what
#at:ifAbsent: does than #scanFor:.  It forces them into the
implementation prematurely, even though OO is supposed to let them
"not care how it works".

Regards,
  Chris

On Fri, Mar 9, 2018 at 7:18 PM,  <commits at source.squeak.org> wrote:
> Eliot Miranda uploaded a new version of Collections to project The Trunk:
> http://source.squeak.org/trunk/Collections-eem.784.mcz
>
> ==================== Summary ====================
>
> Name: Collections-eem.784
> Author: eem
> Time: 9 March 2018, 5:18:51.529302 pm
> UUID: 1862bd2e-3307-4973-b7b1-c8f6ad8d5f53
> Ancestors: Collections-ul.783
>
> Provide more efficient implementation(s) of at:ifPresent:ifAbsent: given impending use in the Compiler.
>
> =============== Diff against Collections-ul.783 ===============
>
> Item was changed:
>   ----- Method: Dictionary>>at:ifPresent:ifAbsent: (in category 'accessing') -----
>   at: key ifPresent: oneArgBlock ifAbsent: absentBlock
> +       "Lookup the given key in the receiver. If it is present, answer the
> +        value of evaluating the oneArgBlock with the value associated
> +        with the key, otherwise answer the value of absentBlock."
> +       ^(array at: (self scanFor: key))
> +               ifNil: [absentBlock value]
> +               ifNotNil: [:association| oneArgBlock value: association value]!
> -       "Lookup the given key in the receiver. If it is present, answer the value of evaluating the oneArgBlock with the value associated with the key, otherwise answer the value of absentBlock."
> -       self at: key ifPresent:[:v| ^oneArgBlock value: v].
> -       ^absentBlock value!
>
> Item was added:
> + ----- Method: WeakIdentityDictionary>>at:ifPresent:ifAbsent: (in category 'accessing') -----
> + at: key ifPresent: oneArgBlock ifAbsent: absentBlock
> +       "Lookup the given key in the receiver. If it is present, answer the
> +        value of evaluating the oneArgBlock with the value associated
> +        with the key, otherwise answer the value of absentBlock."
> +       ^(array at: (self scanFor: key))
> +               ifNil: [absentBlock value]
> +               ifNotNil:
> +                       [:association|
> +                        association == vacuum
> +                               ifTrue: [absentBlock value]
> +                               ifFalse: [oneArgBlock value: association value]]!
>
>