Hi Richard. I agree with your proposal. But it force me to think that we should completely move to SortFunction's. In that case SortCollection will have sortFunction variable instead of sortBlock. And for your scenario reverse operation will be simple expression: "sortFunction := sortFunction reversed".
2018-04-23 3:09 GMT+02:00 Richard O'Keefe <rao...@gmail.com>: > Test case: > #(1 2 4) asSortedCollection reverse add: 3; yourself > <print it> > The answer *should* be aSortedCollection(4 3 2 1) > but *is* aSortedCollection(4 2 1 3). > This works in Squeak. > The problem is that SortedCollection does not define > #reverse[d] but simply inherits it(them), and the > inherited code pays no attention to the sortBlock. > > I propose adding the following two methods to > SortedCollection: > > reverseInPlace > |a z| > a := firstIndex. > z := lastIndex. > [a < z] whileTrue: [array swap: a with: z. a := a + 1. z := z - 1]. > sortBlock := sortBlock > ifNil: [[:x :y | y <= x]] > ifNotNil: [[:x :y | sortBlock value: y value: x]]. > ^self > > reversed > ^self copy reverseInPlace > > The ANSI method is #reverse, not #reversed, but Pharo > defines #reverse to call #reversed, and OrderedCollection overrides > #reversed, so SortedCollection *must* override #reversed. > > #reverseInPlace is the name Squeak uses for the other > method. It also has a definition in SequenceableCollection, which is not > but is > equivalent to > > reverseInPlace > |a z| > a := 1. > z := self size. > [a < z] whileTrue: [self swap: a with: z. a := a + 1. z := z - 1].l > ^self > > r > >
