Please, please, *please* let's not do things like WriteStream on: ''. Just decompile the method and you'll see why. Please, please, *PLEASE* use either WriteStream on: String new or WriteStream on: '' copy. Modifying literals is a baaaad idea.
On Sat, Feb 23, 2013 at 11:49 AM, commits@source.squeak.org wrote:
Nicolas Cellier uploaded a new version of KernelTests to project The Trunk: http://source.squeak.org/trunk/KernelTests-nice.244.mcz
==================== Summary ====================
Name: KernelTests-nice.244 Author: nice Time: 23 February 2013, 8:49:05.862 pm UUID: 7b3445ea-0910-4ee4-a6c7-c7305f930764 Ancestors: KernelTests-fbs.243
Avoid using ReadWriteStream gratuitously when we just need a WriteStream.
=============== Diff against KernelTests-fbs.243 ===============
Item was changed: ----- Method: DateAndTimeEpochTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '1901-01-01T00:00:00+00:00'.ws := '' writeStream.aDateAndTime printOn: ws.self assert: ws contents = ref.ref := 'a TimeZone(ETZ)'.ws := '' writeStream.aTimeZone printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '1901-01-01T00:00:00+00:00'.rw := ReadWriteStream on: ''.aDateAndTime printOn: rw.self assert: rw contents = cs contents.cs := ReadStream on: 'a TimeZone(ETZ)'.rw := ReadWriteStream on: ''.aTimeZone printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: DateAndTimeLeapTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '2004-02-29T13:33:00+02:00'.ws := '' writeStream.aDateAndTime printOn: ws.self assert: ws contents = ref.ref := 'a TimeZone(UTC)'.ws := '' writeStream.aTimeZone printOn: ws.self assert: ws contents = ref !
| cs rw |cs := ReadStream on: '2004-02-29T13:33:00+02:00'.rw := ReadWriteStream on: ''.aDateAndTime printOn: rw.self assert: rw contents = cs contents.cs := ReadStream on: 'a TimeZone(UTC)'.rw := ReadWriteStream on: ''.aTimeZone printOn: rw.self assert: rw contents = cs contents !Item was changed: ----- Method: DateTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '23 January 2004'.ws := '' writeStream.aDate printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '23 January 2004'.rw := ReadWriteStream on: ''.aDate printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: DateTest>>testPrintOnFormat (in category 'testing') ----- testPrintOnFormat
| ref ws |ref :='04*Jan*23'.ws := '' writeStream.aDate printOn: ws format: #(3 2 1 $* 2 2).self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '04*Jan*23'.rw := ReadWriteStream on: ''.aDate printOn: rw format: #(3 2 1 $* 2 2).self assert: rw contents = cs contents!Item was changed: ----- Method: DateTest>>testStoreOn (in category 'testing') ----- testStoreOn
| ref ws |ref := '''23 January 2004'' asDate'.ws := '' writeStream.aDate storeOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '''23 January 2004'' asDate'.rw := ReadWriteStream on: ''.aDate storeOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: DurationTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '1:02:03:04.000000005'.ws := '' writeStream.aDuration printOn: ws.self assert: ws contents = ref!
|cs rw |cs := ReadStream on: '1:02:03:04.000000005'.rw := ReadWriteStream on: ''.aDuration printOn: rw.self assert: rw contents = cs contents.!Item was changed: ----- Method: IntegerTest>>testPrintOnBaseShowRadix (in category 'tests - printing') ----- testPrintOnBaseShowRadix | s |
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 10 showRadix: false. self assert: (s contents = '123').
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 10 showRadix: true. self assert: (s contents = '10r123').
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 8 showRadix: false. self assert: (s contents = '173').
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 8 showRadix: true. self assert: (s contents = '8r173').!Item was changed: ----- Method: StopwatchTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := 'a Stopwatch(suspended:0:00:00:00)'.ws := '' writeStream.aStopwatch printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: 'a Stopwatch(suspended:0:00:00:00)'.rw := ReadWriteStream on: ''.aStopwatch printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: TimeStampTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '2 January 2004 12:34:56 am'.ws := '' writeStream.aTimeStamp printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '2 January 2004 12:34:56 am'.rw := ReadWriteStream on: ''.aTimeStamp printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: TimeStampTest>>testStoreOn (in category 'testing') ----- testStoreOn
| ref ws |ref := '''2 January 2004 12:34:56 am'' asTimeStamp'.ws := '' writeStream.aTimeStamp storeOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '''2 January 2004 12:34:56 am'' asTimeStamp'.rw := ReadWriteStream on: ''.aTimeStamp storeOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: TimespanTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := 'a Timespan(2005-01-01T00:00:00+00:00D7:00:00:00)'.ws := '' writeStream.aTimespan printOn: ws.self assert: ws contents = ref
| cs rw |cs := ReadStream on: 'a Timespan(2005-01-01T00:00:00+00:00D7:00:00:00)'.rw := ReadWriteStream on: ''.aTimespan printOn: rw.self assert: rw contents = cs contentsItem was changed: ----- Method: YearMonthWeekTest>>testMonthPrintOn (in category 'testing') ----- testMonthPrintOn
| aMonth ws |aMonth := Month starting: DateAndTime new duration: 31 days.ws := '' writeStream.aMonth printOn: ws.self assert: ws contents = 'January 1901'.!
| aMonth cs rw |aMonth := Month starting: DateAndTime new duration: 31 days.cs := ReadStream on: 'January 1901'.rw := ReadWriteStream on: ''.aMonth printOn: rw.self assert: rw contents = cs contents.!Item was changed: ----- Method: YearMonthWeekTest>>testYearPrintOn (in category 'testing') ----- testYearPrintOn
| aYear ws |aYear := Year starting: DateAndTime new duration: 365 days.ws := '' writeStream.aYear printOn: ws.self assert: ws contents = 'a Year (1901)'.!
| aYear cs rw |aYear := Year starting: DateAndTime new duration: 365 days.cs := ReadStream on: 'a Year (1901)'.rw := ReadWriteStream on: ''.aYear printOn: rw.self assert: rw contents = cs contents.!
I tried it and did not see a problem. That is an empty string, isn't it? What could go wrong?
On 2/23/13 6:35 PM, Eliot Miranda wrote:
Please, please, *please* let's not do things like WriteStream on: ''. Just decompile the method and you'll see why. Please, please, *PLEASE* use either WriteStream on: String new or WriteStream on: '' copy. Modifying literals is a baaaad idea.
On Sat, Feb 23, 2013 at 11:49 AM, commits@source.squeak.org wrote:
Nicolas Cellier uploaded a new version of KernelTests to project The Trunk: http://source.squeak.org/trunk/KernelTests-nice.244.mcz
==================== Summary ====================
Name: KernelTests-nice.244 Author: nice Time: 23 February 2013, 8:49:05.862 pm UUID: 7b3445ea-0910-4ee4-a6c7-c7305f930764 Ancestors: KernelTests-fbs.243
Avoid using ReadWriteStream gratuitously when we just need a WriteStream.
=============== Diff against KernelTests-fbs.243 ===============
Item was changed: ----- Method: DateAndTimeEpochTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '1901-01-01T00:00:00+00:00'.ws := '' writeStream.aDateAndTime printOn: ws.self assert: ws contents = ref.ref := 'a TimeZone(ETZ)'.ws := '' writeStream.aTimeZone printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '1901-01-01T00:00:00+00:00'.rw := ReadWriteStream on: ''.aDateAndTime printOn: rw.self assert: rw contents = cs contents.cs := ReadStream on: 'a TimeZone(ETZ)'.rw := ReadWriteStream on: ''.aTimeZone printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: DateAndTimeLeapTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '2004-02-29T13:33:00+02:00'.ws := '' writeStream.aDateAndTime printOn: ws.self assert: ws contents = ref.ref := 'a TimeZone(UTC)'.ws := '' writeStream.aTimeZone printOn: ws.self assert: ws contents = ref !
| cs rw |cs := ReadStream on: '2004-02-29T13:33:00+02:00'.rw := ReadWriteStream on: ''.aDateAndTime printOn: rw.self assert: rw contents = cs contents.cs := ReadStream on: 'a TimeZone(UTC)'.rw := ReadWriteStream on: ''.aTimeZone printOn: rw.self assert: rw contents = cs contents !Item was changed: ----- Method: DateTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '23 January 2004'.ws := '' writeStream.aDate printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '23 January 2004'.rw := ReadWriteStream on: ''.aDate printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: DateTest>>testPrintOnFormat (in category 'testing') ----- testPrintOnFormat
| ref ws |ref :='04*Jan*23'.ws := '' writeStream.aDate printOn: ws format: #(3 2 1 $* 2 2).self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '04*Jan*23'.rw := ReadWriteStream on: ''.aDate printOn: rw format: #(3 2 1 $* 2 2).self assert: rw contents = cs contents!Item was changed: ----- Method: DateTest>>testStoreOn (in category 'testing') ----- testStoreOn
| ref ws |ref := '''23 January 2004'' asDate'.ws := '' writeStream.aDate storeOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '''23 January 2004'' asDate'.rw := ReadWriteStream on: ''.aDate storeOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: DurationTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '1:02:03:04.000000005'.ws := '' writeStream.aDuration printOn: ws.self assert: ws contents = ref!
|cs rw |cs := ReadStream on: '1:02:03:04.000000005'.rw := ReadWriteStream on: ''.aDuration printOn: rw.self assert: rw contents = cs contents.!Item was changed: ----- Method: IntegerTest>>testPrintOnBaseShowRadix (in category 'tests - printing') ----- testPrintOnBaseShowRadix | s |
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 10 showRadix: false. self assert: (s contents = '123').
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 10 showRadix: true. self assert: (s contents = '10r123').
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 8 showRadix: false. self assert: (s contents = '173').
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 8 showRadix: true. self assert: (s contents = '8r173').!Item was changed: ----- Method: StopwatchTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := 'a Stopwatch(suspended:0:00:00:00)'.ws := '' writeStream.aStopwatch printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: 'a Stopwatch(suspended:0:00:00:00)'.rw := ReadWriteStream on: ''.aStopwatch printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: TimeStampTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '2 January 2004 12:34:56 am'.ws := '' writeStream.aTimeStamp printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '2 January 2004 12:34:56 am'.rw := ReadWriteStream on: ''.aTimeStamp printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: TimeStampTest>>testStoreOn (in category 'testing') ----- testStoreOn
| ref ws |ref := '''2 January 2004 12:34:56 am'' asTimeStamp'.ws := '' writeStream.aTimeStamp storeOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '''2 January 2004 12:34:56 am'' asTimeStamp'.rw := ReadWriteStream on: ''.aTimeStamp storeOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: TimespanTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := 'a Timespan(2005-01-01T00:00:00+00:00D7:00:00:00)'.ws := '' writeStream.aTimespan printOn: ws.self assert: ws contents = ref
| cs rw |cs := ReadStream on: 'a Timespan(2005-01-01T00:00:00+00:00D7:00:00:00)'.rw := ReadWriteStream on: ''.aTimespan printOn: rw.self assert: rw contents = cs contentsItem was changed: ----- Method: YearMonthWeekTest>>testMonthPrintOn (in category 'testing') ----- testMonthPrintOn
| aMonth ws |aMonth := Month starting: DateAndTime new duration: 31 days.ws := '' writeStream.aMonth printOn: ws.self assert: ws contents = 'January 1901'.!
| aMonth cs rw |aMonth := Month starting: DateAndTime new duration: 31 days.cs := ReadStream on: 'January 1901'.rw := ReadWriteStream on: ''.aMonth printOn: rw.self assert: rw contents = cs contents.!Item was changed: ----- Method: YearMonthWeekTest>>testYearPrintOn (in category 'testing') ----- testYearPrintOn
| aYear ws |aYear := Year starting: DateAndTime new duration: 365 days.ws := '' writeStream.aYear printOn: ws.self assert: ws contents = 'a Year (1901)'.!
| aYear cs rw |aYear := Year starting: DateAndTime new duration: 365 days.cs := ReadStream on: 'a Year (1901)'.rw := ReadWriteStream on: ''.aYear printOn: rw.self assert: rw contents = cs contents.!
On Sat, Feb 23, 2013 at 4:08 PM, Bob Arning arning315@comcast.net wrote:
I tried it and did not see a problem. That is an empty string, isn't it?
Yes, and you're getting away with it because the WriteStream implementation doesn't use become: to grow strings. But back in the day it did and would have resulted in a non-zero string.
What could go wrong?
The method writes to a literal (depending on WriteStream implementation), modifying the method. Try this:
| s | s := WriteStream on: 'hello there'. { thisContext method decompile asString. s nextPutAll: 'BOO!'. thisContext method decompile asString }
you get:
#('DoIt | t1 | t1 := WriteStream on: ''hello there''. ^ {thisContext method decompile asString. t1 nextPutAll: ''BOO!''. thisContext method decompile asString}' 'BOO!' 'DoIt | t1 | t1 := WriteStream on: ''BOO!o there''. ^ {thisContext method decompile asString. t1 nextPutAll: ''BOO!''. thisContext method decompile asString}')
Ouch.
Literals should be immutable. They're not. It requires some VM work to provide per-object write-protection (already done for an old Newspeak interpreter VM) and some image changes. We did this for VisualWorks ten years or so ago and, while it caused some pain in old code, it was a very good thing to have done. For example, DB-mapping systems like GemStone can make excellent use of per-object write-protection.
On 2/23/13 6:35 PM, Eliot Miranda wrote:
Please, please, *please* let's not do things like WriteStream on: ''. Just decompile the method and you'll see why. Please, please, *PLEASE* use either WriteStream on: String new or WriteStream on: '' copy. Modifying literals is a baaaad idea.
On Sat, Feb 23, 2013 at 11:49 AM, commits@source.squeak.org wrote:
Nicolas Cellier uploaded a new version of KernelTests to project The Trunk: http://source.squeak.org/trunk/KernelTests-nice.244.mcz
==================== Summary ====================
Name: KernelTests-nice.244 Author: nice Time: 23 February 2013, 8:49:05.862 pm UUID: 7b3445ea-0910-4ee4-a6c7-c7305f930764 Ancestors: KernelTests-fbs.243
Avoid using ReadWriteStream gratuitously when we just need a WriteStream.
=============== Diff against KernelTests-fbs.243 ===============
Item was changed:
----- Method: DateAndTimeEpochTest>>testPrintOn (in category 'testing')
testPrintOn
| ref ws |ref := '1901-01-01T00:00:00+00:00'.ws := '' writeStream.aDateAndTime printOn: ws.self assert: ws contents = ref.ref := 'a TimeZone(ETZ)'.ws := '' writeStream.aTimeZone printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '1901-01-01T00:00:00+00:00'.rw := ReadWriteStream on: ''.aDateAndTime printOn: rw.self assert: rw contents = cs contents.cs := ReadStream on: 'a TimeZone(ETZ)'.rw := ReadWriteStream on: ''.aTimeZone printOn: rw.self assert: rw contents = cs contents!Item was changed:
----- Method: DateAndTimeLeapTest>>testPrintOn (in category 'testing')
testPrintOn
| ref ws |ref := '2004-02-29T13:33:00+02:00'.ws := '' writeStream.aDateAndTime printOn: ws.self assert: ws contents = ref.ref := 'a TimeZone(UTC)'.ws := '' writeStream.aTimeZone printOn: ws.self assert: ws contents = ref !
| cs rw |cs := ReadStream on: '2004-02-29T13:33:00+02:00'.rw := ReadWriteStream on: ''.aDateAndTime printOn: rw.self assert: rw contents = cs contents.cs := ReadStream on: 'a TimeZone(UTC)'.rw := ReadWriteStream on: ''.aTimeZone printOn: rw.self assert: rw contents = cs contents !Item was changed: ----- Method: DateTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '23 January 2004'.ws := '' writeStream.aDate printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '23 January 2004'.rw := ReadWriteStream on: ''.aDate printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: DateTest>>testPrintOnFormat (in category 'testing') ----- testPrintOnFormat
| ref ws |ref :='04*Jan*23'.ws := '' writeStream.aDate printOn: ws format: #(3 2 1 $* 2 2).self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '04*Jan*23'.rw := ReadWriteStream on: ''.aDate printOn: rw format: #(3 2 1 $* 2 2).self assert: rw contents = cs contents!Item was changed: ----- Method: DateTest>>testStoreOn (in category 'testing') ----- testStoreOn
| ref ws |ref := '''23 January 2004'' asDate'.ws := '' writeStream.aDate storeOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '''23 January 2004'' asDate'.rw := ReadWriteStream on: ''.aDate storeOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: DurationTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '1:02:03:04.000000005'.ws := '' writeStream.aDuration printOn: ws.self assert: ws contents = ref!
|cs rw |cs := ReadStream on: '1:02:03:04.000000005'.rw := ReadWriteStream on: ''.aDuration printOn: rw.self assert: rw contents = cs contents.!Item was changed: ----- Method: IntegerTest>>testPrintOnBaseShowRadix (in category 'tests - printing') ----- testPrintOnBaseShowRadix | s |
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 10 showRadix: false. self assert: (s contents = '123').
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 10 showRadix: true. self assert: (s contents = '10r123').
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 8 showRadix: false. self assert: (s contents = '173').
s := '' writeStream.
s := ReadWriteStream on: ''. 123 printOn: s base: 8 showRadix: true. self assert: (s contents = '8r173').!Item was changed: ----- Method: StopwatchTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := 'a Stopwatch(suspended:0:00:00:00)'.ws := '' writeStream.aStopwatch printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: 'a Stopwatch(suspended:0:00:00:00)'.rw := ReadWriteStream on: ''.aStopwatch printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: TimeStampTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := '2 January 2004 12:34:56 am'.ws := '' writeStream.aTimeStamp printOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '2 January 2004 12:34:56 am'.rw := ReadWriteStream on: ''.aTimeStamp printOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: TimeStampTest>>testStoreOn (in category 'testing') ----- testStoreOn
| ref ws |ref := '''2 January 2004 12:34:56 am'' asTimeStamp'.ws := '' writeStream.aTimeStamp storeOn: ws.self assert: ws contents = ref!
| cs rw |cs := ReadStream on: '''2 January 2004 12:34:56 am'' asTimeStamp'.rw := ReadWriteStream on: ''.aTimeStamp storeOn: rw.self assert: rw contents = cs contents!Item was changed: ----- Method: TimespanTest>>testPrintOn (in category 'testing') ----- testPrintOn
| ref ws |ref := 'a Timespan(2005-01-01T00:00:00+00:00D7:00:00:00)'.ws := '' writeStream.aTimespan printOn: ws.self assert: ws contents = ref
| cs rw |cs := ReadStream on: 'aTimespan(2005-01-01T00:00:00+00:00D7:00:00:00)'.
rw := ReadWriteStream on: ''.aTimespan printOn: rw.self assert: rw contents = cs contentsItem was changed:
----- Method: YearMonthWeekTest>>testMonthPrintOn (in category 'testing')
testMonthPrintOn
| aMonth ws |aMonth := Month starting: DateAndTime new duration: 31 days.ws := '' writeStream.aMonth printOn: ws.self assert: ws contents = 'January 1901'.!
| aMonth cs rw |aMonth := Month starting: DateAndTime new duration: 31 days.cs := ReadStream on: 'January 1901'.rw := ReadWriteStream on: ''.aMonth printOn: rw.self assert: rw contents = cs contents.!Item was changed:
----- Method: YearMonthWeekTest>>testYearPrintOn (in category 'testing')
testYearPrintOn
| aYear ws |aYear := Year starting: DateAndTime new duration: 365 days.ws := '' writeStream.aYear printOn: ws.self assert: ws contents = 'a Year (1901)'.!
| aYear cs rw |aYear := Year starting: DateAndTime new duration: 365 days.cs := ReadStream on: 'a Year (1901)'.rw := ReadWriteStream on: ''.aYear printOn: rw.self assert: rw contents = cs contents.!
On 23-02-2013, at 5:34 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Sat, Feb 23, 2013 at 4:08 PM, Bob Arning arning315@comcast.net wrote:
I tried it and did not see a problem. That is an empty string, isn't it?
Yes, and you're getting away with it because the WriteStream implementation doesn't use become: to grow strings. But back in the day it did and would have resulted in a non-zero string.
If Eliot's explanation doesn't convince you, just consider how ugly an expression it is. Let's have "WriteStream onNewString". Much more readable. Clearly expressed intent.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim "bOtHeR" said Pooh, mistaking the LSD tablet for aspirin
On Sat, Feb 23, 2013 at 10:25 PM, tim Rowledge tim@rowledge.org wrote:
If Eliot's explanation doesn't convince you, just consider how ugly an expression it is. Let's have "WriteStream onNewString". Much more readable. Clearly expressed intent.
I like "String new writeStream" myself.
Colin
I don't understand. There is no become: in SequenceableCollection>>grownBy: So contrarily to st80/VW the literal is not modified.
Personally I prefer String new writeStream. Alternatively, I can use streamContents: which will use a more appropriate initial size...
Nicolas
2013/2/24 Colin Putney colin@wiresong.com:
On Sat, Feb 23, 2013 at 10:25 PM, tim Rowledge tim@rowledge.org wrote:
If Eliot's explanation doesn't convince you, just consider how ugly an expression it is. Let's have "WriteStream onNewString". Much more readable. Clearly expressed intent.
I like "String new writeStream" myself.
Colin
Ugly is the only valid criticism of
WriteStream on: ''
in Squeak, which I thought was the issue originally. Using a non-empty literal is, of course, asking for trouble. And if some code is becoming arguments passed to it from the outside, I think the problem may well lie with the receiver rather than the sender.
Cheers, Bob
On 2/24/13 1:25 AM, tim Rowledge wrote:
On 23-02-2013, at 5:34 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Sat, Feb 23, 2013 at 4:08 PM, Bob Arning arning315@comcast.net wrote:
I tried it and did not see a problem. That is an empty string, isn't it?
Yes, and you're getting away with it because the WriteStream implementation doesn't use become: to grow strings. But back in the day it did and would have resulted in a non-zero string.
If Eliot's explanation doesn't convince you, just consider how ugly an expression it is. Let's have "WriteStream onNewString". Much more readable. Clearly expressed intent.
tim
tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim "bOtHeR" said Pooh, mistaking the LSD tablet for aspirin
2013/2/24 Bob Arning arning315@comcast.net:
Ugly is the only valid criticism of
WriteStream on: ''
in Squeak, which I thought was the issue originally. Using a non-empty literal is, of course, asking for trouble. And if some code is becoming arguments passed to it from the outside, I think the problem may well lie with the receiver rather than the sender.
Ah, OK, I missed the second post of Eliot. Even if we don't become: the literal (our first luck being that #become: is expensive), we modify it if ever it is not empty (our second luck)...
Anyway, my aim was not this specific pattern AnyKindOfStream on: '', or '' writeStream, I think there are about 50 such in the image. This pattern was used before I changed the methods, and I have chosen to not modify too much (I can reconsider if we want to).
My aim was rather to specifically eliminate excessive usage of ReadWriteStream.
A first glimpse at code indicates that we almost never need a ReadWriteStream. 25% of use cases are just using write methods + contents. 74% of use cases are just for a false good reason - optimization. That is: 1) we write 2) we reset 3) we read The optimization is about space: this avoids a copy and a new allocation. Very nice, but for example we could achieve same level of optimization by just replacing the reset send by a readStream send, which would create a well delimited readStream on the target up to writeLimit.
1% of use case, I could not analyze easily, code being too much convoluted ;)
I also saw that some methods are only implemented in ReadWriteStream - like fileOutChanges. I understand why they ended here, for letting an HtmlFileStream prepend a header and append a trailer... But all this is brittle... First, HtmlFileStream sounds weird, why not a HtmlStringStream? Ah yes, we cannot compose Streams, just inheritate... Second, using a low level abstraction - a Stream - and hacking it to handle higher level structures - header, trailer, ... - is exactly the kind of cleverness that makes our simple abstractions become complex and convoluted. I understand that this gave us an HTML output at very low cost at client side (just change name of the stream), and apparently not so high a cost at library side (adding a subclass and a few messages in the hierarchy does not seem very expensive). But the hidden cost when we cripple the Stream library with many of these little brittle API and excessive responsibility, is that it becomes very hard to change/extend the library. We then start adding features to some restricted parts of hierarchy and end up with something that is fragile, non uniform and freezes because no one can modify without breaking something.
Xtreams showed how nice and simple could the Stream library be. I'd like to de-convoluate a bit current code toward that direction. Whether we use Xtreams or just simplify Stream, I don't care, but we have to do something.
Nicolas
Cheers, Bob
On 2/24/13 1:25 AM, tim Rowledge wrote:
On 23-02-2013, at 5:34 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Sat, Feb 23, 2013 at 4:08 PM, Bob Arning arning315@comcast.net wrote:
I tried it and did not see a problem. That is an empty string, isn't it?
Yes, and you're getting away with it because the WriteStream implementation doesn't use become: to grow strings. But back in the day it did and would have resulted in a non-zero string.
If Eliot's explanation doesn't convince you, just consider how ugly an expression it is. Let's have "WriteStream onNewString". Much more readable. Clearly expressed intent.
tim
tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim "bOtHeR" said Pooh, mistaking the LSD tablet for aspirin
I extended WriteStream in VA Smalltalk with:
onStringOfSize: anInt "Answer a new instance to write stream on a string of size anInt."
^self on: (String new: anInt).
so that when I know I need a large stream, I can give it a good starting size.
I do kind of like extending String with writeStream (thinking about it) so one could write:
(String new: anInt) writeStream
Lou
On Sat, 23 Feb 2013 22:25:39 -0800, tim Rowledge tim@rowledge.org wrote:
On 23-02-2013, at 5:34 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Sat, Feb 23, 2013 at 4:08 PM, Bob Arning arning315@comcast.net wrote:
I tried it and did not see a problem. That is an empty string, isn't it?
Yes, and you're getting away with it because the WriteStream implementation doesn't use become: to grow strings. But back in the day it did and would have resulted in a non-zero string.
If Eliot's explanation doesn't convince you, just consider how ugly an expression it is. Let's have "WriteStream onNewString". Much more readable. Clearly expressed intent.
tim
----------------------------------------------------------- Louis LaBrunda Keystone Software Corp. SkypeMe callto://PhotonDemon mailto:Lou@Keystone-Software.com http://www.Keystone-Software.com
squeak-dev@lists.squeakfoundation.org
-
Bob Arning -
Colin Putney -
Eliot Miranda -
Louis LaBrunda -
Nicolas Cellier -
tim Rowledge