isKindOf: in Morphic code...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

isKindOf: in Morphic code...

Eliot Miranda-2
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 ;-)

_,,,^..^,,,_
best, Eliot


Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Tobias Pape

On 04.07.2016, at 22:44, Eliot Miranda <[hidden email]> 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:)


Best regards
        -Tobias
PS: Not advocating anything just reporting what I found
Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

timrowledge

> On 04-07-2016, at 1:59 PM, Tobias Pape <[hidden email]> wrote:
>
>
> On 04.07.2016, at 22:44, Eliot Miranda <[hidden email]> 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:)

It is, and it’s horrible. One of the major things I did to speed up Scratch was fixing hundreds of badly placed isKindOf: sends. Ideally one can find a better algorithm and avoid the issue completely in some places.
Not quite so good is adding lots of isThingummy methods but at least that is faster! This is at least architecturally better since there are plenty of cases where simple class inheritance is not sufficient as a test   because several disjoint classes provide a facility. You *really* don’t want to see multiple isKindOf’s in side an important loop!

I suspect that there are a lot of places where we should *not* simply use the list of submorphs and scan it with isKindOf type tests but instead keep more precise track of relevant submorphs.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Strange OpCodes: SD: Self Destruct



Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Eliot Miranda-2
In reply to this post by Tobias Pape


On Mon, Jul 4, 2016 at 1:59 PM, Tobias Pape <[hidden email]> wrote:

On 04.07.2016, at 22:44, Eliot Miranda <[hidden email]> 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


Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

marcel.taeumel
Eliot Miranda-2 wrote
On Mon, Jul 4, 2016 at 1:59 PM, Tobias Pape <[hidden email]> wrote:

>
> On 04.07.2016, at 22:44, Eliot Miranda <[hidden email]> 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. ;-) 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
Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

marcel.taeumel
marcel.taeumel wrote
Eliot Miranda-2 wrote
On Mon, Jul 4, 2016 at 1:59 PM, Tobias Pape <[hidden email]> wrote:

>
> On 04.07.2016, at 22:44, Eliot Miranda <[hidden email]> 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. ;-) 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
Okay, here is some more constructive feedback on this issue.

The problem is that there are morphs in the world that do not want to be covered: flaps, docking bars, etc. So, it would be good to attach a property to that morph that says "please do not cover me". Then the RealEstateAgent just has to iterate of a world's submorphs once, check for that property and some bounds and we are fine.

No need for #isKindOf:.
No need for #isFlapTab.

Right now, the algorithm is even broken because we assume that all flaps have the same size. For example, there cannot be a bigger icon, a smaller text, or whatever. They all have to have the same size. Kind of unnecessary limitation.

This is the actual "bug".

Best,
Marcel
Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Phil B
Mercel,

On Mon, 2016-07-04 at 14:05 -0700, marcel.taeumel wrote:
> > > > > 

<snip>

> > > > > 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
> Okay, here is some more constructive feedback on this issue.
>
> The problem is that there are morphs in the world that do not want to
> be
> covered: flaps, docking bars, etc. So, it would be good to attach a
> property
> to that morph that says "please do not cover me". Then the
> RealEstateAgent
> just has to iterate of a world's submorphs once, check for that
> property and
> some bounds and we are fine.
>
> No need for #isKindOf:.
> No need for #isFlapTab.
>
> Right now, the algorithm is even broken because we assume that all
> flaps
> have the same size. For example, there cannot be a bigger icon, a
> smaller
> text, or whatever. They all have to have the same size. Kind of
> unnecessary
> limitation.
>
> This is the actual "bug".
>
> Best,
> Marcel
>

It's great that you've eliminated the usage in this specific example
but you are not seeing the forest for the trees.  Eliot's comment
wasn't concerned with what was being tested or why it was being tested,
but rather how it was being tested.  #isKindOf: is testing what exact
kind of thing something is, and its performance implications, rather
than how the thing you are testing behaves.  So if someone were to come
along with MyBetterFlapTab in an alternate class hierarchy, the test as
it current stands will not only perform sub-optimally, but it also
won't behave as expected (i.e. if MyBetterFlapTab implements the
FlapTab protocols it is reasonable to expect an instance of it to work
wherever an instance of FlapTab does)

I think what Eliot was asking was that wherever such tests are being
performed, and for whatever reason, to not use #isKindOf: and use a
protocol-based #is* approach instead that doesn't look at the concrete
class.  Re: the concern about polluting Object, Morph, or whatever...
perhaps using a more generalized #is: approach could help.  Or if it is
such a specialized (i.e. one-off) test that an #is* or #is: check would
not make sense, there is always #respondsTo:

Thanks,
Phil

Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Eliot Miranda-2
In reply to this post by marcel.taeumel


On Mon, Jul 4, 2016 at 1:58 PM, marcel.taeumel <[hidden email]> wrote:
Eliot Miranda-2 wrote
> On Mon, Jul 4, 2016 at 1:59 PM, Tobias Pape &lt;

> Das.Linux@

> &gt; wrote:
>
>>
>> On 04.07.2016, at 22:44, Eliot Miranda &lt;

> eliot.miranda@

> &gt; 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


Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

marcel.taeumel
Eliot Miranda-2 wrote
On Mon, Jul 4, 2016 at 1:58 PM, marcel.taeumel <[hidden email]>
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
Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

marcel.taeumel
marcel.taeumel wrote
Eliot Miranda-2 wrote
On Mon, Jul 4, 2016 at 1:58 PM, marcel.taeumel <[hidden email]>
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
Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Phil B
In reply to this post by marcel.taeumel
Marcel,

On Tue, 2016-07-05 at 00:42 -0700, marcel.taeumel wrote:


>
> @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.
>

It's not an #isClass-check, it's an #isProtocolCollectionCompliant-
check.  It is worse both from a performance and an architecture
standpoint to use #isKindOf:.  I agree that no amount of calling the
'correct' method (if there are such things) makes up for bad design,
but that's not the argument here.

> @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

We'll have to agree to disagree on this point.  Type checking is
fundamentally doing it wrong IMO.

> 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.
>

Don't conflate what these methods are named with the role they perform.
 While many of the #is* methods may appear to be class checks, if used
properly they are actually checks for compliance with collections of
protocols. (this would be easier to see if Smalltalk in general did a
better job of formalizing protocols)  I understand that many
Smalltalkers don't look at them this way, but I would argue that they
should.

Here are a few examples of why the distinction matters:

1) Proxy objects.  If I create a ProtoObject subclass as my proxy
wrapper and pass it into code that does an #isKindOf: check, it will
blow up even though it may be a proxy for exactly the kind of object
being tested for.  If it does an #is* check, that will get forwarded to
the actual object and things will work as expected.  Don't look at what
I am, look at what I tell you I can do!

2) Parallel and/or alternate class hierarchies that are functionally
equivalent or supersets.  Let's say you have created an alternate
hierarchy for Magnitude and its subclasses.  If you implement the
appropriate methods and respond true to #is* (i.e. whatever it is
behaving like), why should (most) receivers of these objects need to
know or care that my MyFractionLike objects aren't actually instances
of Fraction, a subclass of it, or even in the Magnitude hierarchy?

3) A class that is close enough and wants to appear to be, but in
reality is something very different from, the thing it responded true
to for #is*.  I have a number of things that do this in places in the
class hierarchy that make the most sense from an implementation
standpoint, but not necessarily from a taxonomy standpoint. There are
even one or two places in the base Squeak class hierarchy where
classes aren't necessarily where they 'should' be from a taxonomy
standpoint... so be it.

Testing for behavior, not class name or location in the hierarchy, is
the way to go.

Thanks,
Phil

> 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
>

Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Stéphane Rollandin
In reply to this post by marcel.taeumel

> 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.

How fast is #respondsTo:? Can it be used in a tight loop?

Stef


Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

marcel.taeumel
In reply to this post by Phil B
Phil (list) wrote
Marcel,

On Tue, 2016-07-05 at 00:42 -0700, marcel.taeumel wrote:

>
> @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.
>

It's not an #isClass-check, it's an #isProtocolCollectionCompliant-
check.  It is worse both from a performance and an architecture
standpoint to use #isKindOf:.  I agree that no amount of calling the
'correct' method (if there are such things) makes up for bad design,
but that's not the argument here.

> @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

We'll have to agree to disagree on this point.  Type checking is
fundamentally doing it wrong IMO.

> 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.
>

Don't conflate what these methods are named with the role they perform.
 While many of the #is* methods may appear to be class checks, if used
properly they are actually checks for compliance with collections of
protocols. (this would be easier to see if Smalltalk in general did a
better job of formalizing protocols)  I understand that many
Smalltalkers don't look at them this way, but I would argue that they
should.

Here are a few examples of why the distinction matters:

1) Proxy objects.  If I create a ProtoObject subclass as my proxy
wrapper and pass it into code that does an #isKindOf: check, it will
blow up even though it may be a proxy for exactly the kind of object
being tested for.  If it does an #is* check, that will get forwarded to
the actual object and things will work as expected.  Don't look at what
I am, look at what I tell you I can do!

2) Parallel and/or alternate class hierarchies that are functionally
equivalent or supersets.  Let's say you have created an alternate
hierarchy for Magnitude and its subclasses.  If you implement the
appropriate methods and respond true to #is* (i.e. whatever it is
behaving like), why should (most) receivers of these objects need to
know or care that my MyFractionLike objects aren't actually instances
of Fraction, a subclass of it, or even in the Magnitude hierarchy?

3) A class that is close enough and wants to appear to be, but in
reality is something very different from, the thing it responded true
to for #is*.  I have a number of things that do this in places in the
class hierarchy that make the most sense from an implementation
standpoint, but not necessarily from a taxonomy standpoint. There are
even one or two places in the base Squeak class hierarchy where
classes aren't necessarily where they 'should' be from a taxonomy
standpoint... so be it.

Testing for behavior, not class name or location in the hierarchy, is
the way to go.

Thanks,
Phil

> 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
>
Hi Phil,

code lives. It should get improved if new requirements become obvious. #isKindOf: can be a valid (itermediate) step on this road of code evolution. You cannot account for any possible extension points of applications that might be created in the system. This is what refactoring is for.

If an #isClass check is added, it will become part of the interface and callable from any application. If it remains an #isKindOf: check, it might still evolve towards not needing such a check at all. Removing an #isClass check later might break applications that already rely on it.

So, like many parts in the system, using #isKindOf: can be seen as an intermediate step. Now, programmers should be encouraged to also go the next step and think about whether it should become an #isClass check or whether the check is necessary at all. Such a decision cannot be made by a computer progam / automatically. Programmers have to review the code, think about it, decide, and implement/refactor.

Calling this phenomenon a "bug" does not feel right. It feels like a blame on some particular developers. I don't think it is.

We only have so much time.

Best,
Marcel
Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Stéphane Rollandin
> Calling this phenomenon a "bug" does not feel right. It feels like a blame
> on some particular developers. I don't think it is.

+1

Stef


Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Karl Ramberg
A lot of the isKindOf: sends is used to find specific submorphs.

ButtonMorph>>label
| s |
s := ''.
self allMorphsDo: [:m | (m isKindOf: StringMorph) ifTrue: [s := m contents]].
^ s

One could give names to the submorphs as they are initialized, all morphs understand 'name:'

Would that be a solution ?

Best,
Karl




On Tue, Jul 5, 2016 at 1:52 PM, Stéphane Rollandin <[hidden email]> wrote:
Calling this phenomenon a "bug" does not feel right. It feels like a blame
on some particular developers. I don't think it is.

+1

Stef





Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

timrowledge

> On 05-07-2016, at 6:17 AM, karl ramberg <[hidden email]> wrote:
>
> A lot of the isKindOf: sends is used to find specific submorphs.
>
> ButtonMorph>>label
> | s |
> s := ''.
> self allMorphsDo: [:m | (m isKindOf: StringMorph) ifTrue: [s := m contents]].
> ^ s
>
> One could give names to the submorphs as they are initialized, all morphs understand 'name:'
>
> Would that be a solution ?

That’s close to what I was suggesting except that I don’t much like the idea of giving a morph a name; it would be more effective to have an instvar in the *owner* and properly track the owned morph that way. For  example in a ButtonMorph I suggest we should have an instvar ‘label’ and when we set the label morph it should be put in that instvar as well as in the submorphs list. That way we can find it quickly, correctly, without the isKindOf: or even isTextMorph. And after all, both of those prevent a graphic label.


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Useful Latin Phrases:- Oblitus sum perpolire clepsydras! = I forgot to polish the clocks!



Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

timrowledge
In reply to this post by marcel.taeumel

> On 05-07-2016, at 12:45 AM, marcel.taeumel <[hidden email]> wrote:
>
> 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.

I disagree. Not to checking for protocol but to using #respondsTo: - it’s even slower that isKindOf: ! It doesn’t simply climb the class tree checking the class name, it climbs the class tree checking the name of every method in the methodDictionary!

The big issue is the design failure implied by the use of this sort of test; we ought to solve as many of those as possible as soon as possible. Swapping an isKindOf: for some isFoo usage is not ideal by any stretch of the imagination but it can at least solve some important performance problems and it at least allows for multiple classes in disjoint hierarchies to respond sensibly. A better choice of names will often help make life clearer  - for example #canBeButtonMorphLabel would be much better than #isStringMorph. At least that hints at a protocol test rather class membership.

We could, of course, implement a real protocol management & testing capability. At one time I thought that Traits might do that but they went moribund before ever really coming to life.


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
An elephant is a mouse with an operating system.



Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Tobias Pape
In reply to this post by timrowledge

On 05.07.2016, at 18:46, tim Rowledge <[hidden email]> wrote:

>> On 05-07-2016, at 6:17 AM, karl ramberg <[hidden email]> wrote:
>>
>> A lot of the isKindOf: sends is used to find specific submorphs.
>>
>> ButtonMorph>>label
>> | s |
>> s := ''.
>> self allMorphsDo: [:m | (m isKindOf: StringMorph) ifTrue: [s := m contents]].
>> ^ s
>>
>> One could give names to the submorphs as they are initialized, all morphs understand 'name:'
>>
>> Would that be a solution ?
>
> That’s close to what I was suggesting except that I don’t much like the idea of giving a morph a name; it would be more effective to have an instvar in the *owner* and properly track the owned morph that way. For  example in a ButtonMorph I suggest we should have an instvar ‘label’ and when we set the label morph it should be put in that instvar as well as in the submorphs list. That way we can find it quickly, correctly, without the isKindOf: or even isTextMorph. And after all, both of those prevent a graphic label.

What If I drop some other StringMorph onto the Button morph and make it a submorph of it?
It's simple morph composition and the new morph  would just act as label. This is captured by the
behavior above or some protocol-testing methods but not if ButtonMorph itself would manage
what its label morph is. I know this sounds arcane, but Morphic is a very direct-manipulation
environment and a lot of things are determined by structure rather than explicit management.
I find that noteworthy, interesting, and a little bit intriguing. After all, "only" the development
tools make little use of that. It's different in etoys as far as I can tell.

If you make a development tool, its imho wise to maintain all the administrative stuff flying around,
labels, lists, buttons, check boxes, the like. But in "general" Morphic, determination by looking
at what your submorphs are or can do (sic!) seems fine.

Best regards
        -Tobias :)

>
>
> tim




Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Eliot Miranda-2
In reply to this post by marcel.taeumel


On Tue, Jul 5, 2016 at 12:42 AM, marcel.taeumel <[hidden email]> wrote:
Eliot Miranda-2 wrote
> On Mon, Jul 4, 2016 at 1:58 PM, marcel.taeumel &lt;

> Marcel.Taeumel@

> &gt;
> wrote:
>
>> Eliot Miranda-2 wrote
>> > On Mon, Jul 4, 2016 at 1:59 PM, Tobias Pape &lt;
>>
>> > Das.Linux@
>>
>> > &gt; wrote:
>> >
>> >>
>> >> On 04.07.2016, at 22:44, Eliot Miranda &lt;
>>
>> > eliot.miranda@
>>
>> > &gt; 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.

That's simply incorrect, and misunderstands Smalltalk dynamic typing.  And it is precisely in terms of architecture and design that it is worse (although in performance terms it is terrible, especially when disconfirming membership).  Consider Random and ReadStream.

Random inherits from Object, not PositionableStream, yet it provides a subset of the Stream interface via next, next: and next:into:.  PositionableStream provides the layout of inst vars that used to be expected by the VM primitives for next, nextPut: and position that used to be required for performance but no longer are.  So there is no practical benefit to have Random inherit from PositionableStream, but Random *is* a Stream.  So were we to want to test for streamness we *could* implement isStream in both PostionableStream and Random to answer true and in Object to answer false, but we *could not* use aRandom isKindOf: PositionableStream.

So as I stated before, isFoo supports ad-hoc polymorphism, allowing the designer to use inheritance for reuse, and freeing them from (mis) using it for "type".  Whereas isKindOf: restricts designs to increasingly rigid and brittle class hierarchies, which prevent natural Smalltalk style by a broken convention.

So Phil simply misunderstands the issue.  I stand by my assertion, backed by 35 years of Smalltalk programming, that isKindOf: _is a bug_.
 
@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.

isKindOf: is simply lazy.  It's easy to implement the isFoo pattern, and it can be commented properly (for example, the comment can specify exactly which selectors the truth of the message implies).  isKindOf: is quick and dirty, but potentially ambiguous. Precisely what API are we expecting the receiver to exhibit?  Ugh.

 
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.

Agreed.  If isKindOf: can be removed by better design, especially by double dispatching then great.  But in cases where isKindOf: is used exclusively on general instances of some class other than Object (such as Morph), or arguably this and nil as well, replacing isKindOf: invocations can be done without polluting Object, and can provide much better documentation and performance, as well as the key design flexibility it provides.


Best,
Marcel



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




--
_,,,^..^,,,_
best, Eliot


Reply | Threaded
Open this post in threaded view
|

Re: isKindOf: in Morphic code...

Chris Muller-3
In reply to this post by timrowledge
What a great discussion!  Excellent arguments on all sides.

I agree with Marcel that we should not do wholesale conversion of
isKindOf:'s -- they might be able to go away entirely someday by some
factoring or cleanup.

We should just convert them to standard #is* methods lazily, as
tangible, real-world needs to do so arise.

I do think its wrong to say an is* check represents a "design
failure".  Does using #isNil represent a design failure?  No.  There
is nothing wrong with TSTTCPW.  As Marcel said, code lives.

Best,
  Chris


On Tue, Jul 5, 2016 at 12:01 PM, tim Rowledge <[hidden email]> wrote:

>
>> On 05-07-2016, at 12:45 AM, marcel.taeumel <[hidden email]> wrote:
>>
>> 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.
>
> I disagree. Not to checking for protocol but to using #respondsTo: - it’s even slower that isKindOf: ! It doesn’t simply climb the class tree checking the class name, it climbs the class tree checking the name of every method in the methodDictionary!
>
> The big issue is the design failure implied by the use of this sort of test; we ought to solve as many of those as possible as soon as possible. Swapping an isKindOf: for some isFoo usage is not ideal by any stretch of the imagination but it can at least solve some important performance problems and it at least allows for multiple classes in disjoint hierarchies to respond sensibly. A better choice of names will often help make life clearer  - for example #canBeButtonMorphLabel would be much better than #isStringMorph. At least that hints at a protocol test rather class membership.
>
> We could, of course, implement a real protocol management & testing capability. At one time I thought that Traits might do that but they went moribund before ever really coming to life.
>
>
> tim
> --
> tim Rowledge; [hidden email]; http://www.rowledge.org/tim
> An elephant is a mouse with an operating system.
>
>
>

12