Depressions caused by ugly code (was: [FIX] TextStreamFix ([er] questions))

Daniel Vainsencher danielv at netvision.net.il
Wed Jun 11 08:31:20 UTC 2003


[code]
Looks good to me, though I hadn't noticed that was buggy in the first
place, so that's not saying much.

[might write some tests]
That would be cool. Especially since it's starting to look like we'll
have a test-server soon. We could make it run the tests every week/every
batch of updates and send the list a report, letting us know what's
broken immidiately.

Daniel

Boris Gaertner <Boris.Gaertner at gmx.net> wrote:
> Daniel Vainsencher <danielv at netvision.net.il> wrote:
> (in response to a contribution about Text protocol)
> 
> > [everything else about Text operations]
> > Sorry, but I've run out of steam. All that ugly code is depressing me.
> When I received this message, I agreed half-way. This evening,
> after half a day of fighting against a Text problem, I agree fully.
> 
> To tell you the story:
> 
> Text>>
> replaceFrom: start to: stop with: replacement startingAt: repStart 
>  "This destructively replaces elements from start to stop in the
>    receiver starting at index, repStart, in replacementCollection.
>    Do it to both the string and the runs."
> 
>  | rep newRepRuns |
>     rep _ replacement asText. "might be a string"
>    string replaceFrom: start to: stop with: rep string startingAt: repStart.
>    repStart = 1 
>      ifTrue: [runs _ runs copyReplaceFrom: start to: stop with: rep runs]
>      ifFalse:
>       [newRepRuns _ rep runs copyFrom: repStart to: rep size.
>        runs _ runs copyReplaceFrom: start to: stop with: newRepRuns].
> 
> is wrong, but it works for an important special case.
> Have a look at newRepRuns :
> It is assigned a collection that contains the text attributes of the
> replacement text from the first character to be copied into the receiver
> up to the end of the replacement string. This works when 
> rep size - repStart + 1 (the number of characters to be copied from
> rep  into the receiver) is equal to  stop - start + 1 (the number of
> charcters that are replaced in the receiver.)
> 
> I think the method should read: 
> 
> replaceFrom: start to: stop with: replacement startingAt: repStart 
>  "This destructively replaces elements from start to stop
>  in the receiver starting at index, repStart, in replacement
> Collection. Do it to both the string and the runs."
> 
>  | rep newRepRuns |
>     rep _ replacement asText. "might be a string"
>    string replaceFrom: start to: stop with: rep string startingAt: repStart.
>    repStart = 1 
>      ifTrue: [runs _ runs copyReplaceFrom: start
>                                   to: stop
>                                   with: (rep runs copyFrom: 1 to: stop - start + 1)]
>     ifFalse: 
>      [newRepRuns _ rep runs copyFrom: repStart to: repStart + stop - start.
>       runs _ runs copyReplaceFrom: start to: stop with: newRepRuns].
> 
> Here, I assign to newRepRuns  a collection that contains the
> text attributes of the replacement text for the characters that are 
> copied into the receiver.
> My proposal is still not perfect, one should verify that 
> 
> rep size >= repStart + stop - start
> 
>  (repStart + stop - start  is the index of the last character that is
> copied from  rep into the receiver. )
> 
> > How would you feel about doing some groundwork on those classes and
> > their clients, and then writing some tests that check the
> > Text/TextStream API? this would be a good prelude to the kind of deep
> > cleanup these classes are begging for (IMHO).
> I cannot deny that we should have tests. We should also have some
> half-formal description of the classes RunArray and Text. Both
> classes are based on really excellent ideas, but we do not care to explain
> these ideas.
> I will try to write some tests, but this will take a bit more time. Perhaps
> I can also discuss this stuff at CampSmalltalk - a serious CampSmalltalk
> should have a Squeak bug fixing party. I will try to publish a first set
> of tests before CampSmalltalk.
> > 
> > Daniel
> Boris



More information about the Squeak-dev mailing list