[squeak-dev] The Inbox: Collections-EG.908.mcz

David T. Lewis lewis at mail.msen.com
Sat Aug 8 18:40:01 UTC 2020


Hi Levente,

On Sat, Aug 08, 2020 at 06:32:56PM +0200, Levente Uzonyi wrote:
> Hi Dave,
> 
> On Fri, 7 Aug 2020, David T. Lewis wrote:
> 
> >Actually, I think that the new ReadStreamTest>>testPeekBack is not in
> >agreement with the existing RWBinaryOrTextSteamTest>>testPeekBack.
> >
> >The peekBack method was not in Squeak 3.6, but it appears in Squeak 3.8.
> >It is unused in the image, but it presumably is intended to support parsing
> >things from a stream.
> >
> >If I understand correctly, peek means "show me the next element that
> >has not yet been read from the stream", and peekBack means "show me the
> >element before the one that was most recently read from the stream".
> >So I expect that peekBack would mean to skip back 2 from the current
> >position, not skip back 1.
> 
> Another interpretation of peek in the context of streams is to look 
> around the current position without consuming any elements.
> #peek returns what #next would return without consuming any elements,
> and following the same logic, #peekBack is expected to return what #back 
> would without affecting position.
> 
> That interpretation is what Eric implemented.

When I first looked at it, I had the same interpretation as Eric and you.

> 
> If we decide to stick with the previous implementation based on #oldBack, 
> which does not make any practical sense to me, then we'll need another 
> method to get the last read element, because #peekBack would return the 
> penultimate one.
> 

You are right, it is inconsistent and confusing. But I wonder if there is
any use case for a method that just answers the most recently read item.

Sometimes I just look at Cuis when I want to find out how to do things right.
Here is what Cuis does:

  'ABCDEF' readStream next: 2; peekBack. ==> $B
  'ABCDEF' readStream next: 2; back. ==> $B 
  'ABCDEF' readStream next: 2; oldPeekBack. ==> $A 
  
  'ABCDEF' readStream next: 1; peekBack. ==> $A
  'ABCDEF' readStream next: 1; back. $A 
  'ABCDEF' readStream next: 1; oldPeekBack. ==> nil 
  
  'ABCDEF' readStream next: 0; peekBack. ==> Error: CantGoBack
  'ABCDEF' readStream next: 0; back. ==> Error: CantGoBack
  'ABCDEF' readStream next: 0; oldPeekBack. ==> Error: CantGoBack

In Cuis, there are no senders of #peekBack, and one sender of #oldPeekBack
(PositionableStream>>#backChunk).

I think this is consistent with what you are recommending, so maybe we
should just adopt the Cuis implmentations and update our tests accordingly.

Note that the #oldPeekBack in Cuis is like our current #peekBack, and is
still inconsistent with respect to whether it answers nil or raises an error,
which still should be fixed. 

Dave

> 
> Levente
> 
> >
> >Dave
> >
> >On Fri, Aug 07, 2020 at 09:32:56PM -0400, David T. Lewis wrote:
> >>The new tests and fix for PositionableStream look good, but there is a
> >>failure now in RWBinaryOrTextStreamTest>>testPeekBack.
> >>
> >>Dave
> >>
> >>On Sat, Aug 08, 2020 at 12:17:32AM +0000, commits at source.squeak.org wrote:
> >>> A new version of Collections was added to project The Inbox:
> >>> http://source.squeak.org/inbox/Collections-EG.908.mcz
> >>> 
> >>> ==================== Summary ====================
> >>> 
> >>> Name: Collections-EG.908
> >>> Author: EG
> >>> Time: 7 August 2020, 8:17:30.343164 pm
> >>> UUID: d02855a8-a339-4f1d-bede-89a50f07892e
> >>> Ancestors: Collections-eem.907
> >>> 
> >>> Changing  behavior of #peekBack to (correctly) return first element of 
> >>underlying collection when stream position is 1.
> >>> 
> >>> =============== Diff against Collections-eem.907 ===============
> >>> 
> >>> Item was changed:
> >>>   ----- Method: PositionableStream>>peekBack (in category 'accessing') 
> >>-----
> >>>   peekBack
> >>>   	"Return the element at the previous position, without changing 
> >>position.  Use indirect messages in case self is a StandardFileStream."
> >>> - 
> >>>   	| element |
> >>>   	self position = 0 ifTrue: [self errorCantGoBack].
> >>> + 	element := self back.
> >>> - 	self position = 1 ifTrue: [self position: 0.  ^ nil].
> >>> - 	self skip: -2.
> >>> - 	element := self next.
> >>>   	self skip: 1.
> >>>   	^ element!
> >>> 
> >>> 
> >>
> 


More information about the Squeak-dev mailing list