Comment #4 on issue 973 by benjamin...(a)gmail.com: Roassal font organizer
For the fontOrganizerClass, I agree with you. But I feel what you
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
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...
ROPlatform subclasses do:
[ :class |
class fontOrganizerClass superclass == ROPlatform.
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at: