The Trunk: KernelTests-nice.244.mcz
Locked
 
|
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 contents ! Item 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.! |
Re: The Trunk: KernelTests-nice.244.mcz
|
|
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, <[hidden email]> 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 contents > ! > > Item 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.! > > -- best, Eliot |
Re: The Trunk: KernelTests-nice.244.mcz
|
|
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, [hidden email] 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 contents ! Item 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.! |
Re: The Trunk: KernelTests-nice.244.mcz
|
|
On Sat, Feb 23, 2013 at 4:08 PM, Bob Arning <[hidden email]> 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, <[hidden email]> 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 contents > ! > > Item 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.! > > > > > > > > -- best, Eliot |
Re: The Trunk: KernelTests-nice.244.mcz
|
|
On 23-02-2013, at 5:34 PM, Eliot Miranda <[hidden email]> wrote: > On Sat, Feb 23, 2013 at 4:08 PM, Bob Arning <[hidden email]> 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; [hidden email]; http://www.rowledge.org/tim "bOtHeR" said Pooh, mistaking the LSD tablet for aspirin |
Re: The Trunk: KernelTests-nice.244.mcz
|
|
On Sat, Feb 23, 2013 at 10:25 PM, tim Rowledge <[hidden email]> wrote:
I like "String new writeStream" myself. Colin |
Re: The Trunk: KernelTests-nice.244.mcz
|
|
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 <[hidden email]>: > > > > On Sat, Feb 23, 2013 at 10:25 PM, tim Rowledge <[hidden email]> 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 > > > |
Re: The Trunk: KernelTests-nice.244.mcz
|
|
In reply to this post by timrowledge
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 [hidden email] wrote:On Sat, Feb 23, 2013 at 4:08 PM, Bob Arning [hidden email] 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; [hidden email]; http://www.rowledge.org/tim "bOtHeR" said Pooh, mistaking the LSD tablet for aspirin |
Re: The Trunk: KernelTests-nice.244.mcz
|
|
2013/2/24 Bob Arning <[hidden email]>:
> 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 <[hidden email]> wrote: > > On Sat, Feb 23, 2013 at 4:08 PM, Bob Arning <[hidden email]> 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; [hidden email]; http://www.rowledge.org/tim > "bOtHeR" said Pooh, mistaking the LSD tablet for aspirin > > > > > > > > |
The Trunk: KernelTests-nice.244.mcz
|
|
In reply to this post by timrowledge
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 <[hidden email]> wrote: > >On 23-02-2013, at 5:34 PM, Eliot Miranda <[hidden email]> wrote: > >> On Sat, Feb 23, 2013 at 4:08 PM, Bob Arning <[hidden email]> 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:[hidden email] http://www.Keystone-Software.com |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |