[squeak-dev] The Trunk: Graphics-nice.296.mcz
Chris Muller
ma.chris.m at gmail.com
Wed Jul 23 22:02:19 UTC 2014
>>> 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
>
More information about the Squeak-dev
mailing list
|