25 Aug
2013
25 Aug
'13
4:03 p.m.
Status: New
Owner: ----
Labels: Type-Defect Priority-Medium
New issue 973 by benjamin...(a)gmail.com: Roassal font organizer singleton
inconsistencies
http://code.google.com/p/moose-technology/issues/detail?id=973
Following on from Issue 971, use of singletons related to the Roassal font
organizer seems not used consistently.
Using Finder, I've searched all 'Source' of Roassal packages, for the
following both 'ROFontOrganizer' and 'fontOrganizerClass' and found...
----Part1--------------
ROAbstractLabel >> fontFor: anElement with: aCamera
fo := ROPlatform current fontOrganizerClass.
ROAbstractLabel >> offsetWhenDrawing
^ ROPlatform current fontOrganizerClass
ROAbstractLabel >> widthOfLine: string withFont: aFont
^ ROFontOrganizer current widthOfString: string font: font.
I propose the last is changed to...
ROAbstractLabel >> widthOfLine: string withFont: aFont
^ ROPlatform current fontOrganizerClass widthOfString: string font:
font.
and also delete...
ROFontOrganizer >> current
^ self subclasses first
which seems plain wrong.
----Part2--------------
Also I'm a perplexed by the way both of these methods
ROMorphicPlatform>>fontOrganizerClass
^ ROFontOrganizerMorphic
ROPharoAthensPlatform>>fontOrganizerClass
^ ROFontOrganizerAthens
override this one
ROPlatform>>fontOrganizerClass
^ fontOrganizerClass
such that the instance-variable 'fontOrganizerClass' is ignored, as well as
the following seeming irrelevant...
ROPlatform>>fontOrganizerClass:
^ fontOrganizerClass := aClass.
Is ROPlatform ever instantiated, or abstract?
A search for all references to 'ROPlatform' in the 'Source' of all Roassal
finds...
* ROPlatform current (x40)
* ROPlatform platforms (x2)
* ROPlatform removeNamed: (x1)
* ROPlatform add: (x2)
* ROPlatform setCurrent: (x1)
* ROPlatform new (x1)
The last is only in a test ROPlatformTest>>testAddingAPlatform.
I propose to modify...
ROPlatform>>fontOrganizerClass
self subclassResponsibility
delete....
fontOrganizerClass instance-variable
delete...
ROPlatform>>fontOrganizerClass:
from...
ROPlatformTest>>testAddingAPlatform
remove line...
platform fontOrganizerClass: ROFontOrganizer subclasses anyOne.
hope this helps,
cheers -ben
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
25 Aug
25 Aug
4:38 p.m.
New subject: Issue 973 in moose-technology: Roassal font organizer singleton inconsistencies
Comment #1 on issue 973 by benjamin...(a)gmail.com: Roassal font organizer
singleton inconsistencies
http://code.google.com/p/moose-technology/issues/detail?id=973
After making the changes proposed above, all tests are green except
ROPlatformTest>>testAddingAPlatform. The problem is since ROPlatform is
instantiated, then sent to #add: which calls #check which calls
ROPlatform>>fontOrganizerClass which is now subclassResponsibilty.
So further propose adding...
ROPlatform subclass: #ROPlatformSubclassTestResource
and
ROPlatformSubclassTestResource >> fontOrganizerClass
^ ROPlatform subclasses anyOne new fontOrganizerClass.
and modifying ROPlatformTest>>testAddingAPlatform ...
platform := ROPlatformSubclassTestResource new.
This makes all Roassal tests green.
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
29 Aug
29 Aug
4:15 p.m.
New subject: Issue 973 in moose-technology: Roassal font organizer singleton inconsistencies
Updates:
Status: Started
Comment #2 on issue 973 by alexandr...(a)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.
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
7:34 p.m.
New subject: Issue 973 in moose-technology: Roassal font organizer singleton inconsistencies
Comment #3 on issue 973 by benjamin...(a)gmail.com: Roassal font organizer
singleton inconsistencies
http://code.google.com/p/moose-technology/issues/detail?id=973
Any idea how to remove:
ROFontOrganizer >> current
^ self subclasses first
Not sure what you mean. Just delete it :)
Oh, you mean justification...
Use Finder > Choose Packages > "Roassal" only
then search for 'ROFontOrganizer' and you get only three references...
* ROPlatformTest>>testAddingAPlatform
* ROMorphicPlatform>>fontOrganizerClass
* ROPlatform>>check
...none of which send #current to ROFontOrganizer.
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
8:49 p.m.
New subject: Issue 973 in moose-technology: Roassal font organizer singleton inconsistencies
Comment #4 on issue 973 by benjamin...(a)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..."
]
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
13 Nov
13 Nov
10:39 p.m.
New subject: Issue 973 in moose-technology: Roassal font organizer singleton inconsistencies
Updates:
Status: WontFix
Comment #5 on issue 973 by tu...(a)tudorgirba.com: Roassal font organizer
singleton inconsistencies
https://code.google.com/p/moose-technology/issues/detail?id=973
(No comment was entered for this change.)
--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings
3691
days inactive
4136
days old
5 comments
1 participants
participants (1)
-
moose-technology＠googlecode.com