Morphic Cleanup Project (MCP)
Last updated at 12:44 am UTC on 17 January 2006
|CONGRATULATIONS! The first changeset from MCP was included in 3.6alpha! Thanks to all for the hard work (dgd)|
eMail with the original proposal: http://groups.yahoo.com/group/squeak/message/55801
We'll use this page as our workspace. The progress of the project can be followed from here.
We're working in the first step. It means to clean "easy things" (most of them comes from SLint warnings). We'll wait for the finalization of this step before going deeper with the changes.
Next short term goal
- cleanup this page
- re-create the work-environment in a 3.6alpha image
- decide the next steps
- To try all this stuff, use an empty Squeak3.6alpha image.
- Some unittests use the refactoring browser (We'll remove this dependency at the end of the project), so install it using SqueakMap.
- Then install by hand the changeset MCP-0000-CodeAuditingTools-dgd.cs.gz.
- Then evaluate StewardsTools new updateFromMCPSwikiPage. and all the changesets from this page will be installed. It's not magic, it's Smalltalk ;)
- If you want to run the tests you have to load RB (Refactoring Browser) using SqueakMap
Downloads (MCP-number-name-initials.cs.gz !!!)
- Tools (TODO!)
- assorted tools helpful for our process.
- Script to load the needed packages from SqueakMap (TODO!)
- Example: Refactoring Browser
- Script to change the preferences
- Current set of tests (TODO!)
- Have fun with the next refactors
- We'll produce one changeset per fix
- Probably some changes impact on performance. We'll not consider this implications until the finish of the project (make it work, make it right, make it fast!)
- We'll comment (almost?) EVERY method we change
- What we consider Morphs: All classes in categories named "Morphic-something" plus all the subclasses of Morph all over the image (dgd)
List of proposed tasks (please include who proposed the change, so we can ask for details)
Remove hardcoded constants
- AlignmentMorph is supposed to be deprecated, but it's used in several places. Meanwhile we can't remove it, let's clean it. A similar refactoring of #initialize is nedeed (dgd)
- A lot of morphs use the #initialize method to create children morphs. Let's create a method named #initializeChildren (empty on Morph) and let's call if from Morph>>initialize. (dgd)
- Refactor the COMPLETE hierarchy to use the new method.
- Remove Morph>>colorString: and replace the calls (2) to aColor asString (dgd)
- Remove direct reference to class
- please give us more details (dgd)
- I didn't write this originally, but - one of the things that makes Morph the hub of a big tangle, is the fact that it references classes it shouldn't know, for example, alot of it's own subclasses. In another example, Morph should probably not be dependent on FileList. These references should eventually be either moved into a more specialized subclass, moved to a class extension method category (see DVS), or the design changed such that the reference is replaced with a weaker form of coupling. These are medium to large refactorings we should get to eventually (dvf)
Remove FileList reference in Morph class (registering)
Uncoupling from Morph (dgd)
- Removal of symbols used as properties, #raised for example (aoy)
Comment of Classes and methods (dgd)
ProtoMorph/BaseMorph (see it at http://groups.yahoo.com/group/squeak/message/55865) (dgd)
Delegation(Delegate these tasks to other classes)
- Geenie (To an extent, Genie must be coupled to Morph, just like mouse and keyboard events must be supported explicitly by Morph. Genie is defining a new kind of event for Morphs: gesture events. -Lex)
Morph>>delete the last #ifTrue:ifFalse: has the same code in the beginning of both blocks (dgd)
Morph>>allMorphsDo: has an unnecessary size check (dgd)
categorize methods isXXX as testing (dgd)
- Borders(formerly the domain of BorderedMorph)
- It looks like borders have been factored into BorderStyles. Unfortunately they are almost completely undocumented. This may place BorderedMorph into the same category as AlignmentMorph (obsolete but kept around for compatibility.(efc)
create more categories for accessing (ejm: accessing - color, accessing - border, etc) (dgd)
remove unused methods (#addAlarm:after: #addAlarm:at: #addAlarm:with:at: #addAlarm:with:with:after: #addAlarm:with:with:at: #adjustedCenter: #altSpecialCursor0 #altSpecialCursor1 #altSpecialCursor2 #altSpecialCursor3 #balloonColor: #bottomRight: #bounds:from: #completeModificationHash #constructorString #disableGestures #doesOwnRotation #dragSelectionColor #dropSuccessColor #embedInto: #fullLoadCachedState #hasReferencedGestureDictionary #imageFormWithout:andStopThere: #inspectArgumentsPlayerInMorphic: #inspectInMorphic #isBalloonHelp #isModalShell #isSteppingSelector: #morphReport #morphsInFrontOverlapping: #moveWithPenDownByRAA: #nameInModel #okayToExtractEasily #privateFullBounds: #removeAlarm:at: #removeLink: #rootAt: #rootMorphsAtGlobal: #screenLocation #setProperties: #slotSpecifications #specialNameInModel #structureString #submorphBefore #submorphWithProperty: #textToPaste #transportedMorph #updateAllScriptingElements #wantsEveryMouseMove #withAllOwners) (dgd)
SLint new rules or scripts, requests added here: GM SLint Extensions (dgd)
PluggableListMorph and SimpleHierarchicalListMorph common code should be in a superclass (existing or to be created). common code = scrolling, keystroking and more. It was nonexistent in second class, was added by SqueakMap package Hierarchy List Morph - Navigation
Change submorphs to use coordinates in terms of their parent morph. This will reduce the need to keep all the bounds rects of all child morphs updated on a drag or move. This will require updating drawing code too but maybe we can work around this by having the parent morph set up an offset before calling #drawOn:
- I'm not sure if this is a good idea (gm). There are 39 isXXX in 12 categories. Use the following lines to see them:
- isSelectors _ (Morph selectors select: [:each | each beginsWith: 'is' ]).
- isCategories _ isSelectors inject: Set new into: [:sum :each | sum add: (Morph organization categoryOfElement: each); yourself].
- The tradeoff here is when the translating work is done. Currently, translation work must be done for every move, but after that each drawing operation is "free". The proposed scheme would make moves "free" but require translation work on every drawing. Since almost every move requires a redraw, I think the current situation may be the best, but we can continue thinking about it. -efc
- WRONG The drawing operation is not free at all. In fact, even today it uses about everything that you'd need with local coordinates. IOW, the current situation is the worst case in about every respect (-ar).
- Well, the current situation doesn't require point translations on every redraw, does it? A local system would. Intelligent caching could prevent the entire translation chain from being redone, but still you have every single Morph doing translating on every redraw, compounded by the immutable Points and Rectangles that are usually created in great numbers during the translation.
Clean up layout and redraw protocol - how many different kinds of bounds are there and how do they interact? Efforts to add a new kind of layout have had me breaking images (frozen UI) for a week now. This is too fragile.
- please give us more details (dgd)
- Currently, #bounds: results in separate calls to #position and #extent, which do some redundant work (multiple changes/damage reporting). Perhaps this need only be done once?(efc)
Remove direct access of instance variables (proposed by dgd)
- Good idea. (efc) Even if there are no changes, we should provide (good) documentation on the various bounds and changed protocols. A thorough commenting on the entire class Morph won't hurt either. Some of the more common head-scratchers:
- changed, #layoutChanged, #ownerChanged
- bounds, #outerBounds, #fullBounds
Class Comment documentation: I am starting comments for some underdocumented Morphic classes here: Class Comment Drafts (Morph menus). Would you like the finished changesets posted on this page somewhere? Possibly not in the MCP changesets.. they are not tightly coupled, and I don't see much reason to mix them.
WaveEditor #normalize: seems unused. (gm)
TransitionMorph #drawZoomFrameOn: and #drawZoomOn: are almost identical (gm)
KeyboardMorphForInput #emitRest and #mouseDownPitch:event:noteMorph: are almost identical (gm)
MidiInputMorph #atChannel:from:selectInstrument: and ScorePlayerMorph #atTrack:from:selectInstrument: are almost identical
remove "false/true ifTrue:ifFalse:" (old code???), for example PoohTestMorph #drawSubdivisionSpineOn: or RecordingControlsMorph #makeTile (gm)
StringMorph: Get rid of #fontToUse (use #font). Change all the senders to use #font too! (4 or 5 of them) (Morph menus, thanks kc and water)
ScrollPane and TwoWayScrollPane can almost certainly be merged. One would become a "stub" for the other, rather like AlignmentMorph is now. (Morph menus)
Should submorphs be an OrderedCollection instead of an Array ?(efc)
- bounds ==> gm: just search references of bounds? The problem is in the hierarchy...
SystemWindow don't initialize the sizes correctly when big fonts are used (dgd)
Consider use #hasPlayer instead of player == nil (dgd)
Refactor the complete SimpleButtonMorph>>initialize mechanisms. There are some methods #initializeXXX that call super initialize and stuf like that (dgd)
add #beforeResize and #afterResize methods, so Morphs don't override #extent:, leaving it as a simple accessor (gm) (dgd)
- I guess Array is used for performance. We can meassure the performance impact (dgd)
- Array is used for space conservation not performance. Each and every morph can have submorphs and using Array instead of OC saves a lot of space.
refactor color variable and related methods http://groups.yahoo.com/group/squeak/message/58378
B3DSceneExplorerMorph: initialize cries for a refactoring (gm)
BookPageThumbnailMorph: initialize should use self color (gm)
EToyChartMorph, EToyMultiChartMorph: standardBorderColor can be replaced by defaultBorderColor (gm)
EToyProjectRenamerMorph: defaultBorderColor should use self color (gm)
EToyProjectQueryMorph: defaultBorderColor similar to superclass (gm)
EtoyLoginMorph: should be renamed as EToyLoginMorph (gm)
FatBitsPaint: could use self color instead of Color veryVeryLightGray in initialize (gm)
FillInTheBlankMorph: defaultResponseUponCancel for initialize and convertToCurrentVersion:refStream: (gm)
GraphicalDictionaryMenu: initialize needs refactory, it's too long, also parts are similar to GraphicalMenu's initialize (gm)
GraphicalMenu: initialize needs refactory, it's too long (gm)
MidiInputMorph: initialize should use self color (gm)
PasteUpMorph: isWorldMorph should return always false at initialize, so the dependent code makes no sense, or am I missing something? (gm)
PasteUpMorph: should use isWorldMorph instead of checking worldState against nil (gm)
StringMorph: initialize, initializeStandAlone, initializeWithContents:font:emphasis: are almost equal (gm)
SystemWindow: initialize needs refactory, it's too long (gm)
- consider the same for color, borderColor, etc (dgd)
Assigned tasks (please include who is in charge)