On 09.01.2012 00:18, Eliot Miranda wrote:
On Sun, Jan 8, 2012 at 5:46 AM, Mariano Martinez Peck <marianopeck@gmail.com <mailto:marianopeck@gmail.com>> wrote:
Hi guys. Some time ago Henry spot to us a small improvement for WriteStream >> nextPutAll: that we are using in Fuel, but I think it can be general. Henry comment was exactly:
/Also: For variableBytes classes, if you rewrite:
WriteStream nextPutAll: aCollection
| newEnd | *collection class instSpec == aCollection class instSpec ifFalse:* [^ super nextPutAll: aCollection ].
newEnd := position + aCollection size. newEnd > writeLimit ifTrue: [self growTo: newEnd + 10].
collection replaceFrom: position+1 to: newEnd with: aCollection startingAt: 1. position := newEnd.
^ aCollection
You can now pass all variableByte classes (Like ByteString) directly to a stream with a ByteArray collection, and the replaceFrom:to:with:/ /startingAt: primitive will work correctly, just like the file primitive does This means you don't need special Serializers for these either, using f.ex. clunky nextStringPutAll: methods with manual asByteArray conversions./
So...if you agree, I can commit the patch.
Surely this will break special encoded strings where the encode/decode is done in at:/at:put:. Moving the bytes isn't safe in general. However, if the programmer knows they themselves can use next:putAll:startingAt: or some such. I don't think your suggestion is safe in general for WriteStream. Instead if next:putAll:startingAt: doesn't do the job invent some new protocol that will.
In principle I agree, though the only such case I can think of in an image atm is the newly introduced: wa := WordArray new: 2. ws := wa writeStream. ws nextPutAll: 1.0 The string example in itself is rather theoretical, the "convention" is to use ByteStrings for all encodings, and let the programmer fend for himself when it comes to remembering which ByteStrings he's converted to which encoding. (A practice which, sadly, is only enabled by the collection class = aCollection class check (the one in next:putAll:startingAt:), as encoding/decoding a ByteString to a ByteString is much faster than the equivalent operation resulting in a ByteArray... ) So in practice, even the current class check does not protect against what would be the common error. Though, for a general, collection-agnostic approach which can still use primitives between compatible classes, I guess giving the nextPutAll: argument a say in what method is actually used to do a next:putAll:startingAt: would be a better approach than simply removing the check and crossing our fingers. :) Cheers, Henry