2014-07-23 21:42 GMT+02:00 Chris Muller <asqueaker at gmail.com>:

> 2014-07-22 1:29 GMT+02:00 <commits at source.squeak.org>:
>>
>> Nicolas Cellier uploaded a new version of Graphics to project The Trunk:
>>> http://source.squeak.org/trunk/Graphics-nice.296.mcz
>>>
>>> ==================== Summary ====================
>>>
>>> Name: Graphics-nice.296
>>> Author: nice
>>> Time: 22 July 2014, 1:28:48.356 am
>>> UUID: 3f6d869e-9929-4e08-b137-aae22dd3eee6
>>> Ancestors: Graphics-nice.295
>>>
>>> Don't try to accelerate GIF reading by copying the stream in memory.
>>> This should be the purpose of a decent file buffering policy, and if
>>> ever that was not the case, then we should fix the file buffering policy
>>> rather than trying to patch each and every client site.
>>>
>>> Especially when the patch is testing that the stream is in memory with a
>>> class identity to ReadWriteStream!!!
>>> Frankly, how do we know that it's not a RWBinaryOrTextStream ;)
>>>
>>> So why reading an image with a ReadWriteStream?
>>> Oh, sure, this is in ImageReadWriter, so we may need a ReadWriteStream?
>>> No, no, and no!
>>> This thing will either read or write but NEVER read and write.
>>> One simple role, one simple class, no UberPotentSwissKnifeStream with
>>> UberComplexStateToManage (readPosition + writePosition + readLimit +
>>> writeLimit + readBuffer + writeBuffer + readWriteBufferInteraction).
>>>
>>> What's coming next?
>>> Easy to guess, see what (ImageReadWriter>>on:) does frivously to our
>>> stream:
>>> reset??? Hey, ImageReadWriter, why do you take the responsibility to
>>> reset the stream? Did you ask the permission? Did you think of possible
>>> side effects?
>>>
>>
>> the answer is summarized there:
>>
>> readNativeResourceFrom: byteStream
>>     | img aStream |
>>     (byteStream isKindOf: FileStream) ifTrue:[
>>         "Ugly, but ImageReadWriter will send #reset which is implemented
>> as #reopen and we may not be able to do so."
>>         aStream := RWBinaryOrTextStream with: byteStream contents.
>>     ] ifFalse:[
>>         aStream := byteStream.
>>     ].
>>
>> Ugly says it all.
>>
>
> Why not dispatch to byteStream then?
>
>
> Because there is a better solution: remove the undue reset from
ImageReadWriter>>on:
There is no reason why it should reset the stream, that's not its business,
and it won't work with some Stream species.
This reset is just a hack necessary when you use a ReadWriteStream for
reading...
Since people tend to forget it, the reset has been placed at
ReadWriteStream consumption site.
Maybe a better place for reset would be when creating a ReadWriteStream for
reading, but creation message are direction agnostic (do not distinguish
reading from writing usage)...
But if we create a ReadWriteStream for reading and exclusively for reading
- then maybe we'd better just create a ReadStream...
That's all the spirit of my changes.

ReadWriteStream carries complexity, it has both state for reading and for
writing with some unobvious interactions, and requires some hacking like
sending #reset and other niceties.
But 99% of time we are not going to need this complexity, because in most
use cases we will read XOR write, in some cases write THEN read
(ReadWriteStream is just there to avoid a copy), but very very rarely
interleave read and write.
If we just use a ReadStream when we want to read and a WriteStream when we
want to write, we better express our intentions, and we simplify code.
No use for strange state hacking (reset), not use for ugly class testing
(isKindOf:), or its beautified dispatch form...
By the way, how would we name the selector?
I have a very speaking proposition: byteStream
robustifyForThoseClientExpectingAReadWriteStreamAndHackingStreamStateWithAReset.

Nicolas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20140723/5d808ddb/attachment.htm