On 11/10/17, Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com> wrote:
> 2017-11-10 10:53 GMT+01:00 Tobias Pape <Das.Linux at gmx.de>:
>
>>
>> > On 10.11.2017, at 08:00, Nicolas Cellier <nicolas.cellier.aka.nice@
>> gmail.com> wrote:
>> >
>> > But old implementation was a to:do: loop on indices with (self tempAt:
>> > i)
>> > So we already pay the price of low level procedural style,
>> > why having to pay the stream overhead?
>> > It makes code more convoluted than necessary (a higher level dsiguise)
>> >
>> > Or should we rewrite most of SequenceableCollection protocol?
>> >
>>
>> I don't get it.
>>
>> Best regards
>>         -Tobias
>>
>>
> Hi Tobias,
> I say that the main interest of stream is to release you from direct index
> manipulation
> which are considered as lower level implementation details
>
> The code below is just doing a copy.
> It was done with low level index iteration on one side for reading,
> and invoking higher level stream on the other side for writing,
> despite the fact that the low level indices would perfectly match...
>
> We are thus complexifying the code un-necessarily by using two concepts
> where a single one would have been enough.
> Incidentally, code was less efficient, but I don't know if we ever have to
> care (maybe Eliot cares for VM simulation).
>
> So that's why I call this a higher level disguise: it's fake since we
> continue manipulating low level indices.
>
> SequenceableCollection if full of those low level iterations on indices,
> but there it's mainly for the sake of speed/efficiency.
>
> If we have to conceptualize every 4 liners like that, Squeak is really
> going to be conservative ;)

But it means as well that the code is well documented  ...

1. At least in the mailing list here = a traceable record of
development history a.k.a. developer journal.

2. and hopefully also elsewhere such as in method and class comments.

and this results ...

In code reviewed in such a way will also be of very good quality
(peer - reviewed !!)  :-)


>
>> 2017-11-10 2:55 GMT+01:00 Chris Muller <asqueaker at gmail.com>:
>> > +1.  #streamContents: is a high-level message.
>> >
>> > Eliot is a master of C programming, I guess we all see our own style
>> > as less baroque.  I generally try to retain original authors' style
>> > and formatting  as much as possible unless I'm making significant
>> > changes to the method.  I would never only replace someone's
>> > formatting or style with my own.
>> >
>> >
>> > On Tue, Nov 7, 2017 at 11:58 PM, Tobias Pape <Das.Linux at gmx.de> wrote:
>> > >
>> > >> On 08.11.2017, at 02:11, commits at source.squeak.org wrote:
>> > >>
>> > >> Eliot Miranda uploaded a new version of Kernel to project The Trunk:
>> > >> http://source.squeak.org/trunk/Kernel-eem.1117.mcz
>> > >>
>> > >> ==================== Summary ====================
>> > >>
>> > >> Name: Kernel-eem.1117
>> > >> Author: eem
>> > >> Time: 7 November 2017, 5:11:32.043344 pm
>> > >> UUID: 110255e1-5b72-4d99-96bb-e5068c2f5b5b
>> > >> Ancestors: Kernel-tpr.1116
>> > >>
>> > >> Use a slightly less baroque implemetation for Context>>arguments,
>> equivalent to Pharo's.
>> > >>
>> > >
>> > > What is baroque about that?
>> > > I'd prefer the other stile any time.
>> > > I especially find the assignment in the "computation" of the argument
>> ugly (the n assignment) and potentially confusing.
>> > >
>> > > I'm a bit puzzled you prefer the style you just put there to the
>> streaming constructor…
>> > >
>> > > Best regards
>> > >         -Tobias
>> > >
>> > >> =============== Diff against Kernel-tpr.1116 ===============
>> > >>
>> > >> Item was changed:
>> > >>  ----- Method: Context>>arguments (in category 'accessing') -----
>> > >>  arguments
>> > >> +     "Answer the receiver's arguments as an Array.
>> > >> +      We could use simply ^(1 to: self numArgs) collect: [:i| self
>> tempAt: i]
>> > >> +      but for performance and minimality we use the implementation
>> below."
>> > >> +     | n args |
>> > >> +     args := Array new: (n := self numArgs).
>> > >> +     1 to: n do: [:i| args at: i put: (self tempAt: i)].
>> > >> +     ^args!
>> > >> -
>> > >> -     ^ Array new: self numArgs streamContents: [:args |
>> > >> -             1 to: self numArgs do: [: i |
>> > >> -                     args nextPut: (self tempAt: i)]]!
>> > >>
>> > >>
>> > >
>> > >
>> >
>> >
>> >
>>
>>
>>
>