Streams for FileReference in Pharo 7

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Streams for FileReference in Pharo 7

Jan Blizničenko
Hello everyone

I am trying to use Pharo 7 for my project and I noticed that most of my
streams related code does not work anymore. Reason is that API for streams
received from writeStream of FileReference changed and I think it changed
for the worse in my case, because << does not properly handle any other
objects than strings anymore.

In both Pharo 6 and 7 I can write
String streamContents: [ :s | s << 'hello' << 42 << $! ].
and get 'hello42!'.
I can write in Pharo 6 but cannot in Pharo 7
'hello.txt' asFileReference writeStreamDo: [ :s | s << 'hello' << 42 << $! ]
In Pharo 6, I get same result in both examples (written to the file in
second case of course), but in Pharo 7, the second example results in
exception because << does not work with anything other than strings, so it
seems I have to manually convert everything to strings.

This change to streams seems to me like a setback for 2 reasons:
1) API amongst streams is no longer consistent in this case
2) streams for files lost useful functionality and to make my project
compatible with Pharo 7, I have to make lots of effort to make my code
uglier and longer

I would like to ask for the reason of this change and whether is there
anything that can be done with it. Or do I just misuse or misunderstand
anything? Thank you.

Best regards,
Jan



--
Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html

Reply | Threaded
Open this post in threaded view
|

Re: Streams for FileReference in Pharo 7

Sven Van Caekenberghe-2
Hi Jan,

I like #<< too, but I think your assumption that it can print anything on a stream is generally wrong, dangerous and unreliable.

IMHO, you better stick to #print: in that case.

Consider the follow, that all fail:

String streamContents: [ :s | s << 'hello' << 42.5 << $! ].

String streamContents: [ :s | s << 'hello' << (1@2) << $! ].

In other words: #<< works in bizar ways (check the implementors of #putOn:).

I would be inclined to change

ZnEncodedStream>>#<< collection
        ^ self nextPutAll: collection

to

ZnEncodedStream>>#<< collection
        ^ self nextPutAll: collection asString

but it would technically not be the same as the original #<< for in-memory streams.

Opinions ?

Sven

> On 15 Feb 2019, at 09:52, Jan Blizničenko <[hidden email]> wrote:
>
> Hello everyone
>
> I am trying to use Pharo 7 for my project and I noticed that most of my
> streams related code does not work anymore. Reason is that API for streams
> received from writeStream of FileReference changed and I think it changed
> for the worse in my case, because << does not properly handle any other
> objects than strings anymore.
>
> In both Pharo 6 and 7 I can write
> String streamContents: [ :s | s << 'hello' << 42 << $! ].
> and get 'hello42!'.
> I can write in Pharo 6 but cannot in Pharo 7
> 'hello.txt' asFileReference writeStreamDo: [ :s | s << 'hello' << 42 << $! ]
> In Pharo 6, I get same result in both examples (written to the file in
> second case of course), but in Pharo 7, the second example results in
> exception because << does not work with anything other than strings, so it
> seems I have to manually convert everything to strings.
>
> This change to streams seems to me like a setback for 2 reasons:
> 1) API amongst streams is no longer consistent in this case
> 2) streams for files lost useful functionality and to make my project
> compatible with Pharo 7, I have to make lots of effort to make my code
> uglier and longer
>
> I would like to ask for the reason of this change and whether is there
> anything that can be done with it. Or do I just misuse or misunderstand
> anything? Thank you.
>
> Best regards,
> Jan
>
>
>
> --
> Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
>


Reply | Threaded
Open this post in threaded view
|

Re: Streams for FileReference in Pharo 7

alistairgrant
Hi Sven & Jan,

On Fri, 15 Feb 2019 at 10:25, Sven Van Caekenberghe <[hidden email]> wrote:

>
> Hi Jan,
>
> I like #<< too, but I think your assumption that it can print anything on a stream is generally wrong, dangerous and unreliable.
>
> IMHO, you better stick to #print: in that case.
>
> Consider the follow, that all fail:
>
> String streamContents: [ :s | s << 'hello' << 42.5 << $! ].
>
> String streamContents: [ :s | s << 'hello' << (1@2) << $! ].
>
> In other words: #<< works in bizar ways (check the implementors of #putOn:).
>
> I would be inclined to change
>
> ZnEncodedStream>>#<< collection
>         ^ self nextPutAll: collection
>
> to
>
> ZnEncodedStream>>#<< collection
>         ^ self nextPutAll: collection asString
>
> but it would technically not be the same as the original #<< for in-memory streams.
>
> Opinions ?

I've often thought that this change would be nice to have.  #putOn:
doesn't really fit in with writing to text streams (text in the loose
sense of "something that will be read").

I think an argument could be made that #printString or #displayString
should be used, but they both have their own issues in practice.
So...

+1 to "collection asString".

Cheers,
Alistair

Reply | Threaded
Open this post in threaded view
|

Re: Streams for FileReference in Pharo 7

Sven Van Caekenberghe-2


> On 16 Feb 2019, at 14:35, Alistair Grant <[hidden email]> wrote:
>
> Hi Sven & Jan,
>
> On Fri, 15 Feb 2019 at 10:25, Sven Van Caekenberghe <[hidden email]> wrote:
>>
>> Hi Jan,
>>
>> I like #<< too, but I think your assumption that it can print anything on a stream is generally wrong, dangerous and unreliable.
>>
>> IMHO, you better stick to #print: in that case.
>>
>> Consider the follow, that all fail:
>>
>> String streamContents: [ :s | s << 'hello' << 42.5 << $! ].
>>
>> String streamContents: [ :s | s << 'hello' << (1@2) << $! ].
>>
>> In other words: #<< works in bizar ways (check the implementors of #putOn:).
>>
>> I would be inclined to change
>>
>> ZnEncodedStream>>#<< collection
>>        ^ self nextPutAll: collection
>>
>> to
>>
>> ZnEncodedStream>>#<< collection
>>        ^ self nextPutAll: collection asString
>>
>> but it would technically not be the same as the original #<< for in-memory streams.
>>
>> Opinions ?
>
> I've often thought that this change would be nice to have.  #putOn:
> doesn't really fit in with writing to text streams (text in the loose
> sense of "something that will be read").
>
> I think an argument could be made that #printString or #displayString
> should be used, but they both have their own issues in practice.
> So...
>
> +1 to "collection asString".
>
> Cheers,
> Alistair

I have been thinking a bit more about this, and I have a lot of problems with

Integer>>#putOn: aStream
        aStream isBinary
                ifTrue: [ self asByteArray do: [ :each | aStream nextPut: each ] ]
                ifFalse: [ self asString putOn: aStream ]

I think we should simply remove this method. It is ugly and wrong.

#<< should simply be a shortcut for either #nextPut: and #nextPutAll: and nothing more.

The following two are OK for me:

 String streamContents: [ :out | out << 'Hello' << $! ].
 ByteArray streamContents: [ :out | out << #[ 1 2 3 ] << 4 ].

But the following most definitively not:

 (ByteArray streamContents: [ :out | out << 16rFF << 16rFF11 << 16rFFFF22 << 16rFFFFFF33 ]) hex.

When you write out an integer, you have to decide its size, that can never depend on the magnitude (because there is no way to read this back unless there is additional bracketing).

Also, printing to a stream should be (very) efficient. Making copies of objects just for conversion purposes should be avoided.

I think that if we remove Integer>>#putOn: I could live with

 ZnEncodedStream>>#<< anObject
        anObject putOn: self

The other solution is to define #<< as strictly the same as #nextPutAll: and remove all #putOn: implementors.

Sven


Reply | Threaded
Open this post in threaded view
|

Re: Streams for FileReference in Pharo 7

alistairgrant
On Sat, 16 Feb 2019 at 15:49, Sven Van Caekenberghe <[hidden email]> wrote:

>
> > On 16 Feb 2019, at 14:35, Alistair Grant <[hidden email]> wrote:
> >
> > Hi Sven & Jan,
> >
> > On Fri, 15 Feb 2019 at 10:25, Sven Van Caekenberghe <[hidden email]> wrote:
> >>
> >> Hi Jan,
> >>
> >> I like #<< too, but I think your assumption that it can print anything on a stream is generally wrong, dangerous and unreliable.
> >>
> >> IMHO, you better stick to #print: in that case.
> >>
> >> Consider the follow, that all fail:
> >>
> >> String streamContents: [ :s | s << 'hello' << 42.5 << $! ].
> >>
> >> String streamContents: [ :s | s << 'hello' << (1@2) << $! ].
> >>
> >> In other words: #<< works in bizar ways (check the implementors of #putOn:).
> >>
> >> I would be inclined to change
> >>
> >> ZnEncodedStream>>#<< collection
> >>        ^ self nextPutAll: collection
> >>
> >> to
> >>
> >> ZnEncodedStream>>#<< collection
> >>        ^ self nextPutAll: collection asString
> >>
> >> but it would technically not be the same as the original #<< for in-memory streams.
> >>
> >> Opinions ?
> >
> > I've often thought that this change would be nice to have.  #putOn:
> > doesn't really fit in with writing to text streams (text in the loose
> > sense of "something that will be read").
> >
> > I think an argument could be made that #printString or #displayString
> > should be used, but they both have their own issues in practice.
> > So...
> >
> > +1 to "collection asString".
> >
> > Cheers,
> > Alistair
>
> I have been thinking a bit more about this, and I have a lot of problems with
>
> Integer>>#putOn: aStream
>         aStream isBinary
>                 ifTrue: [ self asByteArray do: [ :each | aStream nextPut: each ] ]
>                 ifFalse: [ self asString putOn: aStream ]
>
> I think we should simply remove this method. It is ugly and wrong.
>
> #<< should simply be a shortcut for either #nextPut: and #nextPutAll: and nothing more.
>
> The following two are OK for me:
>
>  String streamContents: [ :out | out << 'Hello' << $! ].
>  ByteArray streamContents: [ :out | out << #[ 1 2 3 ] << 4 ].
>
> But the following most definitively not:
>
>  (ByteArray streamContents: [ :out | out << 16rFF << 16rFF11 << 16rFFFF22 << 16rFFFFFF33 ]) hex.
>
> When you write out an integer, you have to decide its size, that can never depend on the magnitude (because there is no way to read this back unless there is additional bracketing).
>
> Also, printing to a stream should be (very) efficient. Making copies of objects just for conversion purposes should be avoided.
>
> I think that if we remove Integer>>#putOn: I could live with
>
>  ZnEncodedStream>>#<< anObject
>         anObject putOn: self
>
> The other solution is to define #<< as strictly the same as #nextPutAll: and remove all #putOn: implementors.

My thoughts aren't fully formed on this, but:

I think we can break the streams up in to three categories:

- Streams of arbitrary objects, i.e. streams writing to Arrays,
OrderedCollections, etc.
- Streams that serialise the objects, e.g. JSON, STON, ASN.1, etc.
- Streams that write to some form of text output intended for human reading.

We can then group the messages used by each category:

- Arbitrary object: nextPut:, nextPutAll:, putOn:, etc.
- Serialising: stonOn:, etc.
- Text: <<, cr, lf, etc.

Following these guidelines:

- I'd remove the Integer>>putOn: as you suggest.
- Add the #asString to ZnEncodedStream>><< as suggested earlier.

Cheers,
Alistair

Reply | Threaded
Open this post in threaded view
|

Re: Streams for FileReference in Pharo 7

ducasse
In reply to this post by Sven Van Caekenberghe-2
Hi sven

what I do not really with asString is that it potentially created another stream.
with << the implementation of camillo (if I remember correctly) manages correctly both 
single object and collection and it is nice to have. I do not know if his implementation
does not create additional stream.

Stef


On 15 Feb 2019, at 10:25, Sven Van Caekenberghe <[hidden email]> wrote:

Hi Jan,

I like #<< too, but I think your assumption that it can print anything on a stream is generally wrong, dangerous and unreliable.

IMHO, you better stick to #print: in that case.

Consider the follow, that all fail:

String streamContents: [ :s | s << 'hello' << 42.5 << $! ].

String streamContents: [ :s | s << 'hello' << (1@2) << $! ].

In other words: #<< works in bizar ways (check the implementors of #putOn:).

I would be inclined to change

ZnEncodedStream>>#<< collection
^ self nextPutAll: collection

to

ZnEncodedStream>>#<< collection
^ self nextPutAll: collection asString

but it would technically not be the same as the original #<< for in-memory streams.

Opinions ?

Sven

On 15 Feb 2019, at 09:52, Jan Blizničenko <[hidden email]> wrote:

Hello everyone

I am trying to use Pharo 7 for my project and I noticed that most of my
streams related code does not work anymore. Reason is that API for streams
received from writeStream of FileReference changed and I think it changed
for the worse in my case, because << does not properly handle any other
objects than strings anymore.

In both Pharo 6 and 7 I can write
String streamContents: [ :s | s << 'hello' << 42 << $! ].
and get 'hello42!'.
I can write in Pharo 6 but cannot in Pharo 7
'hello.txt' asFileReference writeStreamDo: [ :s | s << 'hello' << 42 << $! ]
In Pharo 6, I get same result in both examples (written to the file in
second case of course), but in Pharo 7, the second example results in
exception because << does not work with anything other than strings, so it
seems I have to manually convert everything to strings.

This change to streams seems to me like a setback for 2 reasons:
1) API amongst streams is no longer consistent in this case
2) streams for files lost useful functionality and to make my project
compatible with Pharo 7, I have to make lots of effort to make my code
uglier and longer

I would like to ask for the reason of this change and whether is there
anything that can be done with it. Or do I just misuse or misunderstand
anything? Thank you.

Best regards,
Jan



--
Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html




Reply | Threaded
Open this post in threaded view
|

Re: Streams for FileReference in Pharo 7

ducasse
In reply to this post by Sven Van Caekenberghe-2

>
> I have been thinking a bit more about this, and I have a lot of problems with
>
> Integer>>#putOn: aStream
> aStream isBinary
> ifTrue: [ self asByteArray do: [ :each | aStream nextPut: each ] ]
> ifFalse: [ self asString putOn: aStream ]
>
> I think we should simply remove this method. It is ugly and wrong.

I agree

> #<< should simply be a shortcut for either #nextPut: and #nextPutAll: and nothing more.
>
> The following two are OK for me:
>
> String streamContents: [ :out | out << 'Hello' << $! ].
> ByteArray streamContents: [ :out | out << #[ 1 2 3 ] << 4 ].
>
> But the following most definitively not:
>
> (ByteArray streamContents: [ :out | out << 16rFF << 16rFF11 << 16rFFFF22 << 16rFFFFFF33 ]) hex.
>
> When you write out an integer, you have to decide its size, that can never depend on the magnitude (because there is no way to read this back unless there is additional bracketing).
>
> Also, printing to a stream should be (very) efficient. Making copies of objects just for conversion purposes should be avoided.

yes.

>
> I think that if we remove Integer>>#putOn: I could live with
>
> ZnEncodedStream>>#<< anObject
> anObject putOn: self

Yes :)
and remove the ugly one? Yes

>
> The other solution is to define #<< as strictly the same as #nextPutAll: and remove all #putOn: implementors.
>
> Sven
>
>



Reply | Threaded
Open this post in threaded view
|

Re: Streams for FileReference in Pharo 7

Sven Van Caekenberghe-2
OK, I that is what I expected.
I will study this again and make a PR with better comments and tests.

> On 18 Feb 2019, at 16:32, ducasse <[hidden email]> wrote:
>
>>
>>
>> I have been thinking a bit more about this, and I have a lot of problems with
>>
>> Integer>>#putOn: aStream
>> aStream isBinary
>> ifTrue: [ self asByteArray do: [ :each | aStream nextPut: each ] ]
>> ifFalse: [ self asString putOn: aStream ]
>>
>> I think we should simply remove this method. It is ugly and wrong.
>
> I agree
>
>> #<< should simply be a shortcut for either #nextPut: and #nextPutAll: and nothing more.
>>
>> The following two are OK for me:
>>
>> String streamContents: [ :out | out << 'Hello' << $! ].
>> ByteArray streamContents: [ :out | out << #[ 1 2 3 ] << 4 ].
>>
>> But the following most definitively not:
>>
>> (ByteArray streamContents: [ :out | out << 16rFF << 16rFF11 << 16rFFFF22 << 16rFFFFFF33 ]) hex.
>>
>> When you write out an integer, you have to decide its size, that can never depend on the magnitude (because there is no way to read this back unless there is additional bracketing).
>>
>> Also, printing to a stream should be (very) efficient. Making copies of objects just for conversion purposes should be avoided.
>
> yes.
>
>>
>> I think that if we remove Integer>>#putOn: I could live with
>>
>> ZnEncodedStream>>#<< anObject
>> anObject putOn: self
>
> Yes :)
> and remove the ugly one? Yes
>
>>
>> The other solution is to define #<< as strictly the same as #nextPutAll: and remove all #putOn: implementors.
>>
>> Sven


Reply | Threaded
Open this post in threaded view
|

Re: Streams for FileReference in Pharo 7

Sven Van Caekenberghe-2
https://github.com/pharo-project/pharo/pull/2695

> On 18 Feb 2019, at 16:52, Sven Van Caekenberghe <[hidden email]> wrote:
>
> OK, I that is what I expected.
> I will study this again and make a PR with better comments and tests.
>
>> On 18 Feb 2019, at 16:32, ducasse <[hidden email]> wrote:
>>
>>>
>>>
>>> I have been thinking a bit more about this, and I have a lot of problems with
>>>
>>> Integer>>#putOn: aStream
>>> aStream isBinary
>>> ifTrue: [ self asByteArray do: [ :each | aStream nextPut: each ] ]
>>> ifFalse: [ self asString putOn: aStream ]
>>>
>>> I think we should simply remove this method. It is ugly and wrong.
>>
>> I agree
>>
>>> #<< should simply be a shortcut for either #nextPut: and #nextPutAll: and nothing more.
>>>
>>> The following two are OK for me:
>>>
>>> String streamContents: [ :out | out << 'Hello' << $! ].
>>> ByteArray streamContents: [ :out | out << #[ 1 2 3 ] << 4 ].
>>>
>>> But the following most definitively not:
>>>
>>> (ByteArray streamContents: [ :out | out << 16rFF << 16rFF11 << 16rFFFF22 << 16rFFFFFF33 ]) hex.
>>>
>>> When you write out an integer, you have to decide its size, that can never depend on the magnitude (because there is no way to read this back unless there is additional bracketing).
>>>
>>> Also, printing to a stream should be (very) efficient. Making copies of objects just for conversion purposes should be avoided.
>>
>> yes.
>>
>>>
>>> I think that if we remove Integer>>#putOn: I could live with
>>>
>>> ZnEncodedStream>>#<< anObject
>>> anObject putOn: self
>>
>> Yes :)
>> and remove the ugly one? Yes
>>
>>>
>>> The other solution is to define #<< as strictly the same as #nextPutAll: and remove all #putOn: implementors.
>>>
>>> Sven
>