Hi Chris,
Yes, you are right, ReadWriteStream looks simple. But it is not.
It inherits from WriteStream which is a Stream that cannot read (self
shouldNotImplement).
WriteStream inherits itself from PositionableStream which can neither read
nor write (self subclassResponsibility).
PositionableStream is rather read-oriented though, see inst.var. readLimit
and its usage which is clearly for delimiting read limit (thus the name).

WriteStream reuse readLimit with a slightly different purpose: indicate the
right-most position written so far.
The purpose is to answer the whole contents even of position has been moved
backward (rather than truncate to current position).
Hence, the natural invariant should be this one:
the readLimit should be incremented when writing pastEnd (past the
right-most position written so far - readLimit).
But it is not... Indeed, primitive: 66 (nextPut:) increments position and
ignores the readLimit, and just consider the writeLimit:
    rw := ReadWriteStream on: (String new: 10).
    rw nextPut: $a.
    rw instVarNamed: #readLimit
PastEnd is interpreted as the physical limit - writeLimit - so as to have
efficient primitives as long as there is room in the target collection.

That doesn't matter, because the position is shared for reading and writing
operations.
Thanks to this property, we can to enforce the invariant differently:
hack every place where we might assign a position backward with a:
    readLimit := readLimit max: position.

The hack is both clever and fragile...
It's a nice hack, because as you observed, there are not so many methods
requiring an overwrite...
But it's fragile, because it means necessary chirurgical operations in
subclasses to maintain the invariant.
And if ever you want to subclass with a Stream maintaining two separate
positions for read and write, boom!
Since this invariant isn't documented anywhere, probably because you just
have to read the code :(
our best way to learn it will be pain: probably a feedback loop valued by
the biological analogy ;)

And heavy chirurgy is what happens in the subclasses zoo which are further
hacked...
The inst. var. position is no more the absolute position in underlying
collection, but rather a relative position in some buffer/segment, both in
case of StandardFileStream and CompressedSourceStream.
So these classes also modify the meaning of readLimit inst. var. to be the
number of bytes readable in the buffer.
And they need yet another way to access the absolute readLimit (endOfFile
for CompressedSourceStream, and OS fseek-ftell-based for
StandardFileStream).

Now, if you are about to modify one of these classes, and try and track
usage of position/readLimit inst. var. you are in brain trouble.
Remember that position and self position might be different things...

So you are somehow right, ReadWriteStream is not at the level of awfullness
I described, but it carries the seeds for this awfullness to be further
developped.
Reusing inst. var. with a different intention across hierarchy is a good
recipe for brain storm (a bug factory and a limitation to extensibility).
Most of the time, we don't need interleaved read/writes, except for a
database backend or a few other stateful cases.
Instead, we mostly write then read. I already replaced a few instances of
ReadWriteStream on this YAGNI principle, and would like this work to be
continued, thus my simplistic reaction.
Of course, your case might differ.

Nicolas

2018-05-02 0:16 GMT+02:00 Chris Muller <asqueaker at gmail.com>:

> C'mon, it has 9 tiny, concise methods, (15 including extensions from
> EToys and Compression) and so only Tim is allowed to characterize that
> as an "awful mess".  :)   No seriously, being the superclass for
> FileStream, it handles all of Squeak's file contents processing and works
> well.
>
> I guess Chris' explanation is a reasonable explanation for such glaring
> inconsistencies in behavior between superclass and subclass, so I
> ended up adding my own #content method which does what I want...
>
>
>
> On Tue, May 1, 2018 at 3:21 PM, Nicolas Cellier
> <nicolas.cellier.aka.nice at gmail.com> wrote:
> > IMO ReadWriteStream is an awfull mess and should better not be used at
> all.
> >
> > 2018-05-01 22:00 GMT+02:00 Chris Muller <ma.chris.m at gmail.com>:
> >>
> >> Does anyone know why ReadWriteStream overrides #contents from
> WriteStream?
> >>
> >> WriteStream behaves as I would expect
> >>
> >>    |stream| stream := WriteStream on: String new.
> >>    stream nextPutAll: 'chris'; reset; nextPutAll: 'C'; contents
>  "--->
> >> 'C'   as expected"
> >>
> >> but ReadWriteStream doesn't...
> >>
> >>    |stream| stream := ReadWriteStream on: String new.
> >>    stream nextPutAll: 'chris'; reset; nextPutAll: 'C'; contents
>  "--->
> >> 'Chris'   unexpected!"
> >>
> >> I want to reuse a ReadWriteStream, so I want #contents to honor the end
> >> position.  What's going on here?
> >>
> >>
> >>
> >
> >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20180502/5a8e39a1/attachment.html>