Hi,
I've just updated to pl2. I've found that neither WORDArray nor SWORDArray implement the required #uncheckedAt:[put:] methods. Also the new version of ExternalArray>>from:to:keysAndValuesDo: checks both bounds before entering the loop. That is inconsistant with all the other implementations of that method (that I've checked) which do not throw a BoundsError until after itterating over whichever elements are legal. E.g: #( 1 2 3 4 ) from: 2 to: 7 keysAndValuesDo: [:k :v | Transcript display: k; cr]. will write 2 3 and 4 to the transcript before throwing. I can see that its been done that way for efficiency, but I suggest that an implementation like (untested): ------------------------------ from: startInteger to: stopInteger keysAndValuesDo: operation "Evaluate the <dyadicValuable>, operation, for each element of the receiver between the <integer> indices, start and stop, inclusive with the element and its index as respectively the second and first arguments." | stop | startInteger > stopInteger ifTrue: [^ self]. startInteger < 1 ifTrue: [self errorSubscriptBounds: startInteger]. startInteger > self size ifTrue: [self errorSubscriptBounds: stopInteger]. "slightly convoluted expression to avoid bounds tests on every iteration" stop := self size min: stopInteger. self uncheckedFrom: startInteger to: stop keysAndValuesDo: operation. stop < stopInteger ifTrue: [self errorSubscriptBounds: stop + 1]. ------------------------------ would be better. -- chris |
"Chris Uppal" <[hidden email]> wrote in message
news:3fb4ec3a$0$107$[hidden email]... > Hi, > > I've just updated to pl2. I've found that neither WORDArray nor SWORDArray > implement the required #uncheckedAt:[put:] methods. Thanks, we'll fix that in PL3, a small patch that we'll be releasing soon, probably next week. > > Also the new version of ExternalArray>>from:to:keysAndValuesDo: checks both > bounds before entering the loop. That is inconsistant with all the other > implementations of that method (that I've checked) which do not throw a > BoundsError until after itterating over whichever elements are legal. > ... Either definition would be compatible with the ANSI definition of the <sequencedReadableCollection> message. If anything I would say the second case is an implementation artefact and arguably less correct, since the whole message is erroneous. Regardless I don't think one should rely on when the error gets raised if that is not explicitly specified. Regards Blair |
Blair McGlashan wrote:
> > Also the new version of ExternalArray>>from:to:keysAndValuesDo: checks > > both bounds before entering the loop. That is inconsistant with all > > the other implementations of that method (that I've checked) which do > > not throw a BoundsError until after itterating over whichever elements > > are legal. ... > > Either definition would be compatible with the ANSI definition of the > <sequencedReadableCollection> message. > If anything I would say the second case is an implementation artefact and > arguably less correct, since the whole message is erroneous. Regardless I > don't think one should rely on when the error gets raised if that is not > explicitly specified. It's no big deal, but it seems to me that consistancy is worthwhile even if the behaviour isn't conforming to a (semi-)formal specification -- the fact that the other implementations (of that message in Dolphin -- not all the other implementations of that message in other Smalltalks) behave in a certain way suggests that that is the way it is *supposed* to behave. But, as I said, it's no big deal, and I already had the code in place to prevent the inconsistancy from "leaking" out from ExternalArray into my own objects' implementations of #from:to:keysAndValuesDo:. -- chris |
"Chris Uppal" <[hidden email]> wrote in message
news:3fb8e826$1$122$[hidden email]... > Blair McGlashan wrote: > > > > Also the new version of ExternalArray>>from:to:keysAndValuesDo: checks > > > both bounds before entering the loop. That is inconsistant with all > > > the other implementations of that method (that I've checked) which do > > > not throw a BoundsError until after itterating over whichever elements > > > are legal. ... > > > > Either definition would be compatible with the ANSI definition of the > > <sequencedReadableCollection> message. > > If anything I would say the second case is an implementation artefact > > arguably less correct, since the whole message is erroneous. Regardless I > > don't think one should rely on when the error gets raised if that is not > > explicitly specified. > > It's no big deal, but it seems to me that consistancy is worthwhile even if the > behaviour isn't conforming to a (semi-)formal specification -- the fact that > the other implementations (of that message in Dolphin -- not all the other > implementations of that message in other Smalltalks) behave in a certain way > suggests that that is the way it is *supposed* to behave. > I don't think I had any such intent when I wrote them. Giving me the benefit of the doubt it might have been because I knew the error would be detected eventually by the #at: check, and I didn't want to duplicate that with another bounds test. On the other hand it might just have been laziness. Let's consider those other implementations anyway. In the base system (ignoring database and COM stuff) there are 4 implementations: SequenceableCollection OrderedCollection Interval ExternalArray Of these, the SequenceableCollection implementation may enumerate entries in the collection before detecting the bounds error. I consider this a "lazy" implementation (you may not agree). OrderedCollection's implementation fails to detect out of bounds errors in certain cases where the elements do not start and stop at the physical bounds of the collection (i.e. the firstIndex > 1 and/or lastIndex < collection basicSize). Interval's implementation fails to detect bounds errors at all. Now the OC and Interval implementations are optimised forms; I think these errors arose because the bounds checks needed by the message are not clearly expressed in the SequenceableCollection implementation, so when it was copied down and optimised the need to then implement bounds checks was overlooked. Its been useful to consider this in detail since doing so lead me to write some tests, at which point I discovered errors in two of the four implementations. I intend to fix these by performing the bounds check up front as in the ExternalArray implementation, which I now consider to be the canonical form. Thanks for bringing it up. Regards Blair |
Blair McGlashan wrote:
> Its been useful to consider this in detail since doing so lead me to write > some tests, at which point I discovered errors in two of the four > implementations. I intend to fix these by performing the bounds check up > front as in the ExternalArray implementation, which I now consider to be > the canonical form. Does that mean that SequencableCollection will (be scheduled to) change too ? I don't really mind either way, as long as I know what I'm supposed to be consistant *with*. -- chris |
"Chris Uppal" <[hidden email]> wrote in message
news:3fba3c87$0$113$[hidden email]... > Blair McGlashan wrote: > > > Its been useful to consider this in detail since doing so lead me to write > > some tests, at which point I discovered errors in two of the four > > implementations. I intend to fix these by performing the bounds check up > > front as in the ExternalArray implementation, which I now consider to be > > the canonical form. > > Does that mean that SequencableCollection will (be scheduled to) change too ? Yes. The cleanest way to fix the bugs is to rename the existing implementations of #from:to:keysAndValuesDo: to #uncheckedFrom:to:keysAndValuesDo: and copy the implementation of #from:to:keysAndValuesDo: from ExternalArray into SequenceableCollection. Regards Blair |
Free forum by Nabble | Edit this page |