Blaine Buxton: Positronic Vibrations From Alto Dorado

I got a comment from Anand that suggested that I change this code:

CommandMessageSend>>queryCommand: query 
 | canSelector |
 canSelector := ('can' , query command selector capitalized) asSymbol.
 ^(self receiver respondsTo: canSelector) 
  ifTrue: 
   [query
    isEnabled: (self receiver perform: canSelector);
    receiver: self receiver.
   true]
  ifFalse: [super queryCommand: query]

to:

CommandMessageSend>>queryCommand: query
 | canSelector isEnabled |
 canSelector := ('can' , query command selector capitalized) asSymbol.
 [isEnabled := self receiver perform: canSelector] 
  on: MessageNotUnderstood 
  do: [:ex | ^super queryCommand: query].
 query
  isEnabled: isEnabled;
  receiver: self receiver.
 ^true

We got rid of the ifTrue:ifFalse: and the code is clearer. But, something bothered me about it. The MessageNotUnderstood could catch any MessageNotUnderstood further down in the stack (it might not be my canCommandSelector). What is a Smalltalker to do? How about this:

Object>>perform: aSelector onNotUnderstoodDo: aBlock
 ^[self perform: aSelector]
  on: MessageNotUnderstood
  do: [:ex | (ex receiver == self and: [ex selector == aSelector]) 
    ifTrue: [aBlock value] 
    ifFalse: [ex pass]]

This message allows us to do the perform, but check to make sure it's the one we care about. Now, our CommandMessageSend code looks like this:

CommandMessageSend>>queryCommand: query
 | canSelector isEnabled |
 canSelector := ('can' , query command selector capitalized) asSymbol.
 isEnabled := self receiver perform: canSelector onNotUnderstoodDo: [^super queryCommand: query].
 query
  isEnabled: isEnabled;
  receiver: self receiver.
 ^true

The code is about the same, but now we are only catching the MessageNotUnderstood exception that we care about and not any others. The code is still clean and we see another example of the power of Smalltalk exceptions. And I would like to thank Anand for the suggestion!

Comments

this looks good and safe.
small nitpick (my apologies for being so)
in onNotUnderstoodDo: the ifTrue: part can be just ifTrue: aBlock rather than ifTrue: [aBlock value]
same result; one less block evaluation. or am I missing something ?
Regards
Anand

By Anand, at 1:13 PM
I actually I thought about it, but decided not to because I've noticed it freaks out some programmers because they're used to seeing that extra block. So, to make it more readable, I optted for the ifTrue: [aBlock value] instead of ifTrue: aBlock instead. You are right though. It is one more block evaluation that is unneeeded. I often err on the side of readability, but it is subjective. Your mileage may vary.

By Blaine, at 5:55 PM
this got me thinking of cases where
[[aBlock value]]value would be different from [aBlock] value.
if i had subclassed Block and if my class had overridden value to say return another block; the above would cause an issue ?

pretty contrived example i know
i shud go do a quick check

By Anonymous, at 10:06 AM
Blaine,

I tried to get this to work with Menus and ContextMenus in the ViewComposer, without success.

Could you give us a for dummies description how to do that?

Thanks,

Günther, guenni

By Günther, at 4:11 PM

Blaine Buxton: Positronic Vibrations From Alto Dorado

My Stuff

Love

Respect

Admiration

My Amps

Links

Archives

Tuesday, September 20, 2005

Re: Dolphin's Command Framework

Comments