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]. Best, Marcel |
On Mon, May 26, 2014 at 02:56:20AM -0700, Marcel Taeumel wrote:
> Should this work? > > OrderedCollection streamContents: [:result | > result nextPutAll: #(a b c) asOrderedCollection]. > It certainly looks like it should work. Camillo Bruni addressed this case in Pharo by intoducing #streamSpecies, which seems like a reasonable approach. Discussion was here: http://forum.world.st/WriteStream-nextPutAll-OrderedCollections-td4634624.html Dave > 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]. > > Best, > Marcel > |
On Mon, May 26, 2014 at 08:15:32AM -0400, David T. Lewis wrote:
> On Mon, May 26, 2014 at 02:56:20AM -0700, Marcel Taeumel wrote: > > Should this work? > > > > OrderedCollection streamContents: [:result | > > result nextPutAll: #(a b c) asOrderedCollection]. > > > > It certainly looks like it should work. > > Camillo Bruni addressed this case in Pharo by intoducing #streamSpecies, which > seems like a reasonable approach. > > Discussion was here: http://forum.world.st/WriteStream-nextPutAll-OrderedCollections-td4634624.html > I tried the Pharo change in Squeak. It is a seemingly small change, but it breaks WriteStreamTest>>testStreamContentsPositioning, which is a serious failure. I think this means that an important performance optimization for String>>streamContents would be broken. Pharo probably does not have that optimization, so the problem would not be noticed there. Dave |
In reply to this post by marcel.taeumel (old)
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 > > Best, > Marcel > > > > -- > View this message in context: http://forum.world.st/streamContents-on-OrderedCollection-broken-tp4760369.html > Sent from the Squeak - Dev mailing list archive at Nabble.com. > > |
On 2014-05-26, at 18:40, Levente Uzonyi <[hidden email]> 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 - Bert - smime.p7s (5K) Download Attachment |
2014-05-26 21:00 GMT+02:00 Bert Freudenberg <[hidden email]>: With current state of code, making it work might have a hard price for low reward IMO.
It would work if only the implementation of Stream was less bloated. But I didn't look at exact details of the failure, maybe adding one more patch will do it... (until the next bug) |
2014-05-26 21:23 GMT+02:00 Nicolas Cellier <[hidden email]>: Note that this (Xtreams) one is working OK:
I see no simple hack at first glance (OrderedCollection new: 3) writing write: #(a b c) asOrderedCollection; conclusion. |
In reply to this post by Bert Freudenberg
On Mon, 26 May 2014, Bert Freudenberg wrote:
> > On 2014-05-26, at 18:40, Levente Uzonyi <[hidden email]> 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 - > > > |
Free forum by Nabble | Edit this page |