Marcel Taeumel uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-mt.1491.mcz ==================== Summary ==================== Name: Morphic-mt.1491 Author: mt Time: 11 July 2019, 7:43:26.133838 am UUID: 8e7d61cb-067a-d145-a86e-3484bfb38617 Ancestors: Morphic-cmfcmf.1490 Replaces one use of Symbol class >> #lookup: with the more common (i.e., more senders) idiom Symbol class >> #hasInterned:ifTrue:. =============== Diff against Morphic-cmfcmf.1490 =============== Item was changed: ----- Method: SearchBar>>smartSearch:in: (in category 'searching') ----- smartSearch: text in: morph "Take the user input and perform an appropriate search" | input newContents | self removeResultsWidget. input := text asString ifEmpty:[^self]. self class useSmartSearch ifFalse: [^ ToolSet default browseMessageNames: input]. "If it is a global or a full class name, browse that class." (Smalltalk bindingOf: input) ifNotNil:[:assoc| | global | global := assoc value. ^ToolSet browse: (global isBehavior ifTrue:[global] ifFalse:[global class]) selector: nil]. "If it is a symbol and there are implementors of it, browse those implementors." + Symbol hasInterned: input ifTrue: [:selector | - (Symbol lookup: input) ifNotNil: [:selector | (SystemNavigation new allImplementorsOf: selector) ifNotEmpty:[:list| ^SystemNavigation new browseMessageList: list name: 'Implementors of ' , input]]. "If it starts uppercase, browse classes if any. Otherwise, just search for messages." input first isUppercase ifTrue: [ (UIManager default classFromPattern: input withCaption: '') ifNotNil:[:aClass| ^ToolSet browse: aClass selector: nil] ifNil: [ newContents := input, ' -- not found.'. self searchTerm: newContents. self selection: (input size+1 to: newContents size). self currentHand newKeyboardFocus: morph textMorph. ^ self]] ifFalse: [ ToolSet default browseMessageNames: input].! |
Hi Marcel,
These changes should go the other way around. #hasInterned:ifTrue: is private and more specific. #lookup: is more general (#hasInterned:ifTrue: uses lookup) and faster. In my opinion, #hasInterned:ifTrue: should be deprecated, then removed. Levente On Thu, 11 Jul 2019, [hidden email] wrote: > Marcel Taeumel uploaded a new version of Morphic to project The Trunk: > http://source.squeak.org/trunk/Morphic-mt.1491.mcz > > ==================== Summary ==================== > > Name: Morphic-mt.1491 > Author: mt > Time: 11 July 2019, 7:43:26.133838 am > UUID: 8e7d61cb-067a-d145-a86e-3484bfb38617 > Ancestors: Morphic-cmfcmf.1490 > > Replaces one use of Symbol class >> #lookup: with the more common (i.e., more senders) idiom Symbol class >> #hasInterned:ifTrue:. > > =============== Diff against Morphic-cmfcmf.1490 =============== > > Item was changed: > ----- Method: SearchBar>>smartSearch:in: (in category 'searching') ----- > smartSearch: text in: morph > "Take the user input and perform an appropriate search" > | input newContents | > self removeResultsWidget. > input := text asString ifEmpty:[^self]. > self class useSmartSearch ifFalse: [^ ToolSet default browseMessageNames: input]. > > "If it is a global or a full class name, browse that class." > (Smalltalk bindingOf: input) ifNotNil:[:assoc| | global | > global := assoc value. > ^ToolSet browse: (global isBehavior ifTrue:[global] ifFalse:[global class]) selector: nil]. > > "If it is a symbol and there are implementors of it, browse those implementors." > + Symbol hasInterned: input ifTrue: [:selector | > - (Symbol lookup: input) ifNotNil: [:selector | > (SystemNavigation new allImplementorsOf: selector) ifNotEmpty:[:list| > ^SystemNavigation new > browseMessageList: list > name: 'Implementors of ' , input]]. > > "If it starts uppercase, browse classes if any. Otherwise, just search for messages." > input first isUppercase > ifTrue: [ > (UIManager default classFromPattern: input withCaption: '') > ifNotNil:[:aClass| ^ToolSet browse: aClass selector: nil] > ifNil: [ > newContents := input, ' -- not found.'. > self searchTerm: newContents. > self selection: (input size+1 to: newContents size). > self currentHand newKeyboardFocus: morph textMorph. > ^ self]] > ifFalse: [ > ToolSet default browseMessageNames: input].! |
Hi Levente, hmmm... yet, the "ifNil:"-check needed for #lookup: impedes readability in my opinion. :-) Best, Marcel
|
What about adding #lookup:ifFound:?
|
I put a proposal in the inbox: Collections-mt.840 Best, Marcel
|
Hi Marcel,
I see no problem with ifNotNil: here. If I wanted to create a simpler API, I'd implement something like this: String >> #lookupSymbol ^Symbol lookup: self Then I could write 'foo' lookupSymbol ifNotNil: [ :fooSymbol | ... ] instead of (Symbol lookup: 'foo') ifNotNil: [ :fooSymbol | ... ] or Symbol lookup: 'foo' ifFound: [ :fooSymbol | ... ] But given how rarely symbol lookups happen in practice, I'd keep things simple and fast. Levente On Thu, 11 Jul 2019, Marcel Taeumel wrote: > I put a proposal in the inbox: Collections-mt.840 > Best, > Marcel > > Am 11.07.2019 12:41:06 schrieb Marcel Taeumel <[hidden email]>: > > What about adding #lookup:ifFound:? > > Am 11.07.2019 11:47:37 schrieb Marcel Taeumel <[hidden email]>: > > Hi Levente, > hmmm... yet, the "ifNil:"-check needed for #lookup: impedes readability in my opinion. :-) > > Best, > Marcel > > Am 11.07.2019 11:02:13 schrieb Levente Uzonyi <[hidden email]>: > > Hi Marcel, > > These changes should go the other way around. #hasInterned:ifTrue: is > private and more specific. #lookup: is more general (#hasInterned:ifTrue: > uses lookup) and faster. > In my opinion, #hasInterned:ifTrue: should be deprecated, then removed. > > Levente > > On Thu, 11 Jul 2019, [hidden email] wrote: > > > Marcel Taeumel uploaded a new version of Morphic to project The Trunk: > > http://source.squeak.org/trunk/Morphic-mt.1491.mcz > > > > ==================== Summary ==================== > > > > Name: Morphic-mt.1491 > > Author: mt > > Time: 11 July 2019, 7:43:26.133838 am > > UUID: 8e7d61cb-067a-d145-a86e-3484bfb38617 > > Ancestors: Morphic-cmfcmf.1490 > > > > Replaces one use of Symbol class >> #lookup: with the more common (i.e., more senders) idiom Symbol class >> #hasInterned:ifTrue:. > > > > =============== Diff against Morphic-cmfcmf.1490 =============== > > > > Item was changed: > > ----- Method: SearchBar>>smartSearch:in: (in category 'searching') ----- > > smartSearch: text in: morph > > "Take the user input and perform an appropriate search" > > | input newContents | > > self removeResultsWidget. > > input := text asString ifEmpty:[^self]. > > self class useSmartSearch ifFalse: [^ ToolSet default browseMessageNames: input]. > > > > "If it is a global or a full class name, browse that class." > > (Smalltalk bindingOf: input) ifNotNil:[:assoc| | global | > > global := assoc value. > > ^ToolSet browse: (global isBehavior ifTrue:[global] ifFalse:[global class]) selector: nil]. > > > > "If it is a symbol and there are implementors of it, browse those implementors." > > + Symbol hasInterned: input ifTrue: [:selector | > > - (Symbol lookup: input) ifNotNil: [:selector | > > (SystemNavigation new allImplementorsOf: selector) ifNotEmpty:[:list| > > ^SystemNavigation new > > browseMessageList: list > > name: 'Implementors of ' , input]]. > > > > "If it starts uppercase, browse classes if any. Otherwise, just search for messages." > > input first isUppercase > > ifTrue: [ > > (UIManager default classFromPattern: input withCaption: '') > > ifNotNil:[:aClass| ^ToolSet browse: aClass selector: nil] > > ifNil: [ > > newContents := input, ' -- not found.'. > > self searchTerm: newContents. > > self selection: (input size+1 to: newContents size). > > self currentHand newKeyboardFocus: morph textMorph. > > ^ self]] > > ifFalse: [ > > ToolSet default browseMessageNames: input].! > > > |
Hi Levente, I agree. There are only like 40 uses if such explicit symbol lookup. In most cases #asSymbol or the use of strings works just fine. No need to expand the Symbol interface here. +1 for #lookupSymbol Best, Marcel
|
Free forum by Nabble | Edit this page |