[squeak-dev] #streamContents: on OrderedCollection broken?

Levente Uzonyi leves at elte.hu
Mon May 26 21:12:01 UTC 2014


On Mon, 26 May 2014, Bert Freudenberg wrote:

>
> On 2014-05-26, at 18:40, Levente Uzonyi <leves at elte.hu> wrote:
>
>> On Mon, 26 May 2014, Marcel Taeumel wrote:
>>
>>> Should this work?
>>>
>>> OrderedCollection streamContents: [:result |
>>> 	result nextPutAll: #(a b c) asOrderedCollection].
>>>
>>> Well, this works:
>>>
>>> OrderedCollection streamContents: [:result |
>>> 	result nextPutAll: #(a b c)].
>>>
>>> And this works, too:
>>>
>>> OrderedCollection streamContents: [:result |
>>> 	result nextPut: #foo; nextPutAll: #(a b c) asOrderedCollection].
>>
>> Even if it would work, it's a bad pattern, don't use it. OrderedCollection implements its own streaming API (#add:, #addAll:), which is a lot more efficient. I would use the same API as in streams in my own Smalltalk, but that's another story.
>>
>> Also, converting Arrays to OrderedCollections just to throw them away in the next step is just as bad as creating a stream over an OrderedCollection. Don't do it.
>>
>>
>> Levente
>
> Still, it should work, don't you think?

Or just raise an error. Oh, wait, that's already the case, it just needs a 
better error message. :)

The cause of the problem is that WriteStream assumes that the underlying 
collection has the required number of slots exposed by #at:, #at:put: and 
#size. This is not the case with OrderedCollection because it uses another 
collection and hides its internals.

So if you just want to make it work - and let the bad practice spread - 
then just replace #new: with #ofSize: when the collection is (re)created 
in WriteStream (and superclasses if any).

For example changing #new: to #ofSize: in WriteStream >> #growTo: makes 
the first snippet work.
The same can be done in SequenceableCollection >> #new:streamContents: to 
decrease the number of unnecessary allocations by one.

But this fix won't work for other SequenceableCollections like LinkedList
or Heap (which shouldn't really be a SequenceableCollection anyway).
So I think the right thing to do is to implement #new:streamContents: in 
these classes (including OrderedCollection) as

 	self shouldNotImplement


Levente

>
> - Bert -
>
>
>


More information about the Squeak-dev mailing list