Updates: Status: Started

Comment #2 on issue 973 by alexandr...@gmail.com: Roassal font organizer singleton inconsistencies http://code.google.com/p/moose-technology/issues/detail?id=973

Hi Ben,

Thanks for having a look at this. Indeed, the platform management is not the best. I have introduced your first change:

ROAbstractLabel >> widthOfLine: string withFont: aFont ^ ROPlatform current fontOrganizerClass widthOfString: string font: font.

Any idea how to remove: ROFontOrganizer >> current ^ self subclasses first ?

For the fontOrganizerClass, I agree with you. But I feel what you propose a rather ad-hoc. We need to rethink completely the platform implementation I guess.

Comment #4 on issue 973 by benjamin...@gmail.com: Roassal font organizer singleton inconsistencies http://code.google.com/p/moose-technology/issues/detail?id=973

For the fontOrganizerClass, I agree with you. But I feel what you propose a rather ad-hoc. We need to rethink completely the platform implementation I guess.

What is it that you don't like? ROPlatformSubclassTestResource ?

Its easier to pick holes than to fix them, but as a rough guess from the outside, ROPlatform seems like it was originally designed to be instance-based, storing different platform configurations as instance-objects, but was later implemented more class-based using polymorphic-constants. I would never have considered the latter prior to using Smalltalk, but I'm tending to like the approach. It seems more intention revealing and visible to have it in code rather than hidden as data inside an instance-object.

So the big question is which approach is preferred. If the latter, maybe the same should be considered for the other instance-variables of ROPlatform (as seen in ROPlatform>check.) Then maybe you don't need ROPlatform>>add:, hence not need ROPlatform>>check, hence not need my proposed ROPlatformSubclassTestResource.

The only alternatives I can see are:

1. Every instance-method of ROMorphicPlatform and ROPharoAthensPlatform returns a constant. So if you prefer the instance-based way, then both of these classes should be deleted.

2. I tried altering ROPlatformTest>>testAddingAPlatform to have... platform := ROPlatform subclasses anyOne new. but that fails the test differently, since the line... platform name: 'test' is irrelevant, and #name returns a constant in the subclasses, and in ROPlatform>>add: it gets stored for instance in the dictionary at 'morphic' rather than 'test'.

3. If you go with the class-base polymorphic-constants, then delete ROPlatform_class>>add: and hence ROPlatformTest>>testAddingAPlatform (as well as ROPlatform>>check), and do the check as a test like...

ROPlatformTest>>testValidPlatforms ROPlatform subclasses do: [ :class | class fontOrganizerClass superclass == ROPlatform. "etc..." ]