>>> 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.

Okay, I thought you wrote that method and hated it because you were
"forced" to use "isKindOf: FileStream".  Hence my suggestion that
could easily be cleaned with a dispatch, but now I see you're
commenting about a much broader ugliness; one in the design and/or
usage of ReadWriteStream.  Okay.

> 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.

Well, I don't know about the 99%, but as long as you acknowledge that
there ARE use-cases needing interleaved reads and writes.  _Magma_ for
example!  :)  Another is the .changes file, right?

> 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.

Yes, when there is ONLY a need to read, a ReadStream is appropriate, a
ReadWriteStream, not.  When there is a need for reading and writing, a
ReadWriteStream is appropriate.

> 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
>