The comment for OrderedCollection>before:ifAbsent: states, in part, "Answer the result of evaluating the exceptionBlock if ... there are no elements before it."
The code, however, performs self errorFirstObject: target if the object is the first element of the collection.
Which should be changed: the code or the comment?
Jim
Of Jim Menard
The comment for OrderedCollection>before:ifAbsent: states, in part, "Answer the result of evaluating the exceptionBlock if ... there are no elements before it."
The code, however, performs self errorFirstObject: target if the object is the first element of the collection.
Which should be changed: the code or the comment?
Patch 6159 in 3.9 has the fix (this can also be applied to 3.7) SequenceableCollection>>before: target ifAbsent: exceptionBlock "Answer the receiver's element immediately before target. Answer the result of evaluating the exceptionBlock if target is not an element of the receiver, or if there are no elements before it."
| index | index _ self indexOf: target. ^ (index == 0 or: [index == 1]) ifTrue: [exceptionBlock value] ifFalse: [self at: index - 1]
On Apr 30, 2005, at 12:05 PM, Thomas Koenig wrote:
Patch 6159 in 3.9 has the fix (this can also be applied to 3.7) SequenceableCollection>>before: target ifAbsent: exceptionBlock "Answer the receiver's element immediately before target. Answer the result of evaluating the exceptionBlock if target is not an element of the receiver, or if there are no elements before it."
| index | index _ self indexOf: target. ^ (index == 0 or: [index == 1]) ifTrue: [exceptionBlock value] ifFalse: [self at: index - 1]
Thank you. A question: how was it decided that the comment was correct and the code was wrong? I ask for two reasons, one practical and one philosophical: (1) Will it break anybody's code to change this code? (2) Is there a unit test that covers this, and if so what was it testing?
(The topic of code "versus" comments has been discussed this past week on ruby-talk.)
Jim
Jim Menard
On Apr 30, 2005, at 12:05 PM, Thomas Koenig wrote:
Patch 6159 in 3.9 has the fix (this can also be applied to 3.7) SequenceableCollection>>before: target ifAbsent: exceptionBlock "Answer the receiver's element immediately before target. Answer the result of evaluating the exceptionBlock if target is not an element of the receiver, or if there are no elements before it."
| index | index _ self indexOf: target. ^ (index == 0 or: [index == 1]) ifTrue: [exceptionBlock value] ifFalse: [self at: index - 1]
Thank you. A question: how was it decided that the comment was correct and the code was wrong?
Can't really give you much of the history on this. I can say that IMO the intention revealing selector was congruent with the comment and not the code.
I ask for two reasons, one practical and one philosophical: (1) Will it break anybody's code to change this code?
Again don't know. Note there is code such at the Traits package that won't work w/o the fix.
(2) Is there a unit test that covers this, and if so what was it testing?
Not that I know of. Unfortunately we are still a way from where we rely on unit tests to drive or even validate out code fixes.
squeak-dev@lists.squeakfoundation.org