On Tue, 9 Oct 2001, Tim Rowledge wrote:

> I think you'll find that your code
>
> >	!OrderedCollection methodsFor: 'copying' stamp: 'sac 10/5/2001 12:07'!
> >	copyFrom: startIndex to: endIndex
> >		"Answer a copy of the receiver that contains elements from position
> >		startIndex to endIndex."
> >		| targetCollection j |
> >		endIndex < startIndex
> >			ifTrue: [^ self species new: 0].
> >		targetCollection _ self species new
> >					setContents: (Array new: endIndex + 1 - startIndex).
> >		j _ 0.
> >		^self species new
> >					setContents: (array copyFrom: startIndex + firstIndex - 1 to: endIndex + firstIndex - 1)
> ... is rather incorrect, too.



> 'targetCollection' and 'j#'are not used at all. 'self species new' will
> create an OrderedCollection of size 10 and make an Array of size 10
> that you immediately throw away. Very wasteful.

Doh, yes. I must have not thought to remove the extra code...

Where is the other incorrect code? I think its right assuming that this
code should not be responsible for detecting out-of bounds values and/or
other errors and giving them special behaivor.


> Something more like 'self species basicNew setContents:' would be
> closer.  However, note that you would then have to implement extra

Thanks; I hadn't realized this side of it.

> versions of #copyFrom:to: for various subclasses of OrderedCollection to
> compensate for this; when the current method loops adding each element
> to the new collection many things are taken care of by the new
> collection. By fiddling around very explicitly you lose that safety net.

Well, copyFrom:To: appears to be sorta broken; how do you define indexes
in a SortedCollection? Perhaps its misnamed and should have been called
copyFromIndex:ToIndex:?

Thanks for pointing out this brokenness in my code. But, I don't think
its much more broken than what used to be there..

>
> I note that SortedCollection already needs some work to reimplement
> #copyFrom:to: etc to deal with the sortBlock instvar. And just how can
> we implement SortedCollection>reversed? Or #copyReplaceFrom:to:with: ?
> Aside of course from 'self shouldNotImplement'....

Yep. Maybe now would be a good time for someone to just gut out
SortedCollection and replace it with the skiplist code someone contributed
earlier?

Scott