On 2 January 2014 22:39, David T. Lewis <lewis at mail.msen.com> wrote:
> On Thu, Jan 02, 2014 at 09:54:59PM +0000, Frank Shearar wrote:
>> On 2 January 2014 20:54,  <commits at source.squeak.org> wrote:
>> > David T. Lewis uploaded a new version of Collections to project The Inbox:
>> > http://source.squeak.org/inbox/Collections-dtl.556.mcz
>> >
>> > ==================== Summary ====================
>> >
>> > Name: Collections-dtl.556
>> > Author: dtl
>> > Time: 2 January 2014, 3:54:39.379 pm
>> > UUID: c115b70b-d18c-4f60-b218-2f48af79d653
>> > Ancestors: Collections-fbs.555
>> >
>> > Simplify RunArray class>>scanFrom: by arranging for TextAttribute to create the appropriate instance. Remove explicit references to TextAttribute subclasses from RunArray.
>> >
>> > TextSqkProjectLink is a text attribute that controls project entry and is thus associated with Project.. Move it from Collections-Text to System-Support.
>> >
>> > =============== Diff against Collections-fbs.555 ===============
>> >
>> > Item was changed:
>> > + ----- Method: PluggableTextAttribute>>actOnClickFor: (in category 'event handling') -----
>> > - ----- Method: PluggableTextAttribute>>actOnClickFor: (in category 'clicking') -----
>> >   actOnClickFor: anObject
>> >         evalBlock ifNil: [ ^self ].
>> >         evalBlock numArgs = 0 ifTrue: [ evalBlock value.  ^true ].
>> >         evalBlock numArgs = 1 ifTrue: [ evalBlock value: anObject.  ^true ].
>> >         self error: 'evalBlock should have 0 or 1 arguments'!
>> >
>> > Item was changed:
>> >   ----- Method: RunArray class>>scanFrom: (in category 'instance creation') -----
>> >   scanFrom: strm
>> >         "Read the style section of a fileOut or sources file.  nextChunk has already been done.  We need to return a RunArray of TextAttributes of various kinds.  These are written by the implementors of writeScanOn:"
>> >         | runs values attrList char |
>> >         (strm peekFor: $( ) ifFalse: [^ nil].
>> >         runs := OrderedCollection new.
>> >         [strm skipSeparators.
>> >          strm peekFor: $)] whileFalse:
>> >                 [runs add: (Number readFrom: strm)].
>> >         values := OrderedCollection new.        "Value array"
>> >         attrList := OrderedCollection new.      "Attributes list"
>> > +       [(char := strm peek) == nil] whileFalse: [
>> > +               char isSeparator
>> > +                       ifTrue: [strm next "space, cr do nothing"]
>> > +                       ifFalse: [char == $,
>> > +                                       ifTrue: [strm next.
>> > +                                               values add: attrList asArray.
>> > +                                               attrList := OrderedCollection new]
>> > +                                       ifFalse: [attrList add:  (TextAttribute newFrom: strm)]
>> > +                               ]
>>
>> Ah yes, that's a much better idea. I mean, it is a registry of sorts,
>> but I must have been blind not to see this. Nice!
>>
>
> But I also used a #caseOf elsewhere in the code. Mainly just to annoy people
> who think that it is "not object oriented"  ;-)

Ah phooey, I actually missed that (the chance to say "yay #caseOf:
!"). The #caseOf: shows exactly why I like the sender: it lets you
remove all of the if-then-else boilerplate. And, unlike farming out
all that work - two tokens each! - to the subclasses, you get to see
the entire algorithm in 10 lines.

> If noone thinks it is too horrible, I'll move these changes to trunk in a
> day or two. I might first do a bit more cleanup of method categories in the
> TextAttribute classes and see if I can take care of the rest of the uncategorized
> things.

+1 from me!

> I also have to say that your work to reduce package dependencies seems to have
> the nice side effect of encouraging cleaner organization of the code. In many
> cases we have dependencies that exist for good reason, but it seems that we
> also have a lot of dependencies that exist more or less by accident, and are
> clues that something might be better organized in the code.

I'd love to claim "of course!" :) Seriously, I didn't expect to
uncover, er, warts and bumps, in the process of sorting out the
dependencies. It does encourage me to keep on untangling things
though.

frank

> Dave