[squeak-dev] Re: isKindOf: in Morphic code...

marcel.taeumel Marcel.Taeumel at hpi.de
Tue Jul 5 07:45:50 UTC 2016


marcel.taeumel wrote
> 
> Eliot Miranda-2 wrote
>> On Mon, Jul 4, 2016 at 1:58 PM, marcel.taeumel <

>> Marcel.Taeumel@

>> >
>> wrote:
>> 
>>> Eliot Miranda-2 wrote
>>> > On Mon, Jul 4, 2016 at 1:59 PM, Tobias Pape <
>>>
>>> > Das.Linux@
>>>
>>> > > wrote:
>>> >
>>> >>
>>> >> On 04.07.2016, at 22:44, Eliot Miranda <
>>>
>>> > eliot.miranda@
>>>
>>> > > wrote:
>>> >>
>>> >> > Hi All, Hi Marcel,
>>> >> >
>>> >> >     when I see code like this, and there's a lot of it in Morphic,
>>> >> >
>>> >> > !Flaps class methodsFor: 'testing' stamp: 'mt 5/17/2016 14:17'!
>>> >> > anyFlapsVisibleIn: aWorld
>>> >> >
>>> >> >         aWorld submorphsDo: [:m |
>>> >> >                 (m isKindOf: FlapTab) ifTrue: [^ true]].
>>> >> >
>>> >> >         ^ false! !
>>> >> >
>>> >> > I think this is performance thrown on the floor (isKindOf: is
>>> awfully
>>> >> slow, especially in huge hierarchies like Morphic, and bad design,
>>> >> restricting one to a concrete class).  And I think that Morph
>>> provides a
>>> >> perfect place to put an extension that doesn't pollute Object.  So I
>>> >> would
>>> >> like to see
>>> >> >
>>> >> > anyFlapsVisibleIn: aWorld
>>> >> >
>>> >> >         aWorld submorphsDo:
>>> >> >                [:m| m isFlapTab ifTrue: [^true]].
>>> >> >         ^ false! !
>>> >> >
>>> >> > with the emphasis on isFlapTab ;-)
>>> >>
>>> >> Well, class testing seems to be a Morphic pattern, given #findA:
>>> (alias
>>> >> #submorphOfClass:)
>>> >>
>>> >
>>> > I don't care.  It's *WRONG*.  isKindOf: is a _bug_.
>>> >
>>> >
>>> >>
>>> >> Best regards
>>> >>         -Tobias
>>> >> PS: Not advocating anything just reporting what I found
>>> >>
>>> >
>>> >
>>> >
>>> > --
>>> > _,,,^..^,,,_
>>> > best, Eliot
>>>
>>> Hey Eliot,
>>>
>>> no, it's not a bug. ;-)
>> 
>> 
>> Yes, isKindOf: *is* a bug, at least in the usage of determining supported
>> messages.  Smalltalk has ad-hoc polymorphism that allows any class to
>> implement and set of messages, without having to inherit from a
>> superclasss
>> that defines that set of messages.  isKindOf: breaks this property; it
>> restricts the designer to using a concrete class, in violation of the
>> language's flexibility.  isKindOf: is therefore most definitely a bug.
>> 
>> One might think there are valid uses when editing a class hierarchy
>> programmatically.  But for this there are messages understood by classes,
>> not instances, such as includesBehavior:, inheritsFrom: etc.  These are
>> legitimate.  But isKindOf: is not.  It is a horrible hack.
>> 
>> Although, there is room for improvement. #isFlapTab
>>> sounds good. Note that this is not even a critical piece of code because
>>> is
>>> only called if you open a system window, for example.
>>>
>>> Try [Flaps anyFlapsVisibleIn: ActiveWorld] bench. It's fast enough. A
>>> microsecond here, having around 30 windows open. So, you will not notice
>>> a
>>> delay. Still, you're right. We should reduce the usage of #isKindOf:.
>>>
>>> Not polluting Object means still polluting Morph.
>>>
>>> (Yeah, maybe we should use slower computers to program.)
>>>
>>> Checks we should consider to add or use more often if it is already
>>> there:
>>> #isCornerGripMorph
>>> #isProportionalSplitterMorph
>>> #isCompiledMethod
>>> #isColor
>>> #isInfiniteForm
>>>
>>> ... still, a quick look through the usage of #isKindOf: in Morphic code
>>> revealed no serious performance issues. We should refactor MenuMorph to
>>> not
>>> need #isKindOf: that often. This might also be true for other places.
>>>
>>> The pattern "isClass" is not always a good solution for many cases in
>>> the
>>> system.
>>>
>>> Best,
>>> Marcel
>>>
>>>
>>>
>>> --
>>> View this message in context:
>>> http://forum.world.st/isKindOf-in-Morphic-code-tp4904890p4904895.html
>>> Sent from the Squeak - Dev mailing list archive at Nabble.com.
>>>
>>>
>> 
>> 
>> -- 
>> _,,,^..^,,,_
>> best, Eliot
> @Phil: #isKindOf: is no better or worse than the #isClass-check. Maybe
> performance-wise, but arguably not in terms of architecture and code
> design. While #isClass might be a little closer to idiomatic Smalltalk
> code, it should never be used as an excuse for bad design.
> 
> @all: I do see beauty in the pattern of asClass/isClass. For example,
> asSet/isSet, asOrderedCollection/isOrderedCollection. 
> 
> However, anytime a programmer tends to use a type-check and there is no
> "isClass" available, I would rather suggest him to use #isKindOf: for the
> moment instead of adding another two methods to encode the "isClass" in
> both base class and subclass. Always think twice before blowing up a
> class' interface. It impedes code readability. Especially if we are
> talking about a base class that is already that big, like Morph. Then, if
> there is really no other choice -- design-wise -- one has to consider
> performance. #isKindOf: is expensive. Okay. Then you can easily add the
> isClass-check. Fine.
> 
> Sorry, but this is absolutely no black-white decision as Eliot and Phil
> proposed. There are many factors to consider. Semantics, code/interface
> readability, performance, idioms.
> 
> In many cases -- if not all -- I suspect a design issue behind the use of
> #isKindOf: or #isClass. Still, I do like #respondsTo: because it is
> specific to a scenario/domain and leverages the dynamic capabilities of
> Smalltalk. For example, "model respondsTo: #foobar" as an extension point
> in graphical widgets.
> 
> We should *not* batch-convert all uses of #isKindOf: to an #isClass check
> but use our Senders-Tools to hunt down all the design issues left in the
> system.
> 
> Best,
> Marcel

One more thing: If you really consider to implement the "isClass" check in
two unrelated branches of an inheritance tree, you arguably have a design
issue. Just use #respondsTo:. For example, TranscriptStream should not
implement #isModel but rather implement the Model interface as needed.
#respondsTo: will work fine.

Best,
Marcel



--
View this message in context: http://forum.world.st/isKindOf-in-Morphic-code-tp4904890p4904937.html
Sent from the Squeak - Dev mailing list archive at Nabble.com.


More information about the Squeak-dev mailing list