Chris Muller uploaded a new version of CollectionsTests to project The Inbox:
http://source.squeak.org/inbox/CollectionsTests-cmm.312.mcz ==================== Summary ==================== Name: CollectionsTests-cmm.312 Author: cmm Time: 4 June 2019, 3:32:07.434178 pm UUID: b63a7b5c-085b-4479-bc8d-8910d2afeeaf Ancestors: CollectionsTests-mt.311 Additional test for #joinSeparatedBy: describes the expected behavior for unordered collections. =============== Diff against CollectionsTests-mt.311 =============== Item was added: + ----- Method: CollectionTest>>testJoin (in category 'tests') ----- + testJoin + | joined | + self assert: #(a b c d e) join = 'abcde'. + self assert: (#(a b c) joinSeparatedBy: '|') = 'a|b|c'. + joined := (#(a b c) asSet) joinSeparatedBy: '|'. + self assert: (#(2 4) allSatisfy: [ : index | (joined at: index) = $| ]). + self assert: (#(1 3 5) allSatisfy: [ : index | 'abc' includes: (joined at: index) ])! Item was removed: - ----- Method: SequenceableCollectionTest>>testJoin (in category 'tests - converting') ----- - testJoin - - self assert: #(a b c d e) join = 'abcde'. - self assert: (#(a b c) joinSeparatedBy: '|') = 'a|b|c'.! |
Note that joined='a|a|a' would pass the test. A longer joined too. Le mar. 4 juin 2019 à 22:32, <[hidden email]> a écrit : Chris Muller uploaded a new version of CollectionsTests to project The Inbox: |
Okay. Fixed in CollectionsTests-cmm.313.mcz.
On Tue, Jun 4, 2019 at 4:13 PM Nicolas Cellier <[hidden email]> wrote: > > Note that joined='a|a|a' would pass the test. A longer joined too. > > Le mar. 4 juin 2019 à 22:32, <[hidden email]> a écrit : >> >> Chris Muller uploaded a new version of CollectionsTests to project The Inbox: >> http://source.squeak.org/inbox/CollectionsTests-cmm.312.mcz >> >> ==================== Summary ==================== >> >> Name: CollectionsTests-cmm.312 >> Author: cmm >> Time: 4 June 2019, 3:32:07.434178 pm >> UUID: b63a7b5c-085b-4479-bc8d-8910d2afeeaf >> Ancestors: CollectionsTests-mt.311 >> >> Additional test for #joinSeparatedBy: describes the expected behavior for unordered collections. >> >> =============== Diff against CollectionsTests-mt.311 =============== >> >> Item was added: >> + ----- Method: CollectionTest>>testJoin (in category 'tests') ----- >> + testJoin >> + | joined | >> + self assert: #(a b c d e) join = 'abcde'. >> + self assert: (#(a b c) joinSeparatedBy: '|') = 'a|b|c'. >> + joined := (#(a b c) asSet) joinSeparatedBy: '|'. >> + self assert: (#(2 4) allSatisfy: [ : index | (joined at: index) = $| ]). >> + self assert: (#(1 3 5) allSatisfy: [ : index | 'abc' includes: (joined at: index) ])! >> >> Item was removed: >> - ----- Method: SequenceableCollectionTest>>testJoin (in category 'tests - converting') ----- >> - testJoin >> - >> - self assert: #(a b c d e) join = 'abcde'. >> - self assert: (#(a b c) joinSeparatedBy: '|') = 'a|b|c'.! >> >> > |
Hi Chris, I did not mean we need a longer joined example. I did mean that the test omitted to check joined length, so an implementation returning 'a|b|c|' would pass... Also each of abc should be found in joined rather than each of joined found in abc, otherwise 'a|a|a' would pass... I fail to see how you changed that in next version, but it's late, so i have an excuse ;) Also, for the bag case, i would expect to find the 5 z in joined, and i do not see it in the new version of the test. IMO, the unordered case is going to need a separate method, because it already takes too much place relatively to more usual sequenceable case. The difficulty to write these tests, and relative length of sequenceable/unordered cases speak by themself: it perfectly illustrate non obvioussness of the feature, that's why i asked. So yes, we can easily implement the feature, but should we? IMO, we ain't gonna need it, or so rarely... To make more sense out of the test, we could select joined at odd indices, sorted, and check that it equals original collection sorted (because it's like testing sequenceable shuffled join:)... Le mar. 4 juin 2019 à 23:55, Chris Muller <[hidden email]> a écrit : Okay. Fixed in CollectionsTests-cmm.313.mcz. |
Le mer. 5 juin 2019 à 00:29, Nicolas Cellier <[hidden email]> a écrit :
Or better, joined substrings sorted equals: (original collect: asString) sorted.
|
In reply to this post by Nicolas Cellier
Hi Nicolas,
The original test didn't test that either, but I went ahead and added it into CollectionsTests-cmm.315.
CollectionsTests-cmm.315 now covers that. All green.
With this new test, I hope you're convinced otherwise now.
They were _not_ difficult to write at all. Nicolas, I wrote and improved them in near real-time to your suggestions! Now they're more robust than they were before. Your #sorted comparison was genius. If there's any confusion left, Marcel's original comment says all there is to say: "Returns a string, which is a concatenation of each element's string representation separated by another string." "Each element's". That's it. It says _nothing_ about sequence. It also basically duplicates Goran's original Collection>>#asStringOn:delimiter:, which has never been questioned. On SequenceableCollection is questionable, IMO.
We absolutely should. I'm going to want it when I get to converting Magma <--> GraphQL query's at performance. - Chris |
On Tue, Jun 4, 2019 at 9:55 PM Chris Muller <[hidden email]> wrote:
> > Hi Nicolas, > >> >> >> I did mean that the test omitted to check joined length, so an implementation returning 'a|b|c|' would pass... > > > The original test didn't test that either, but I went ahead and added it into CollectionsTests-cmm.315. Err, I guess it did, implicitly, as does the new one. |
Free forum by Nabble | Edit this page |