Issue 2756: Morphic Cleansing - Remove use of findA:

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

Issue 2756: Morphic Cleansing - Remove use of findA:

Guillermo Polito
Hi, removing some findA: message sendings i've found:

MenuMorph>>doButtonAction
    "Do the receiver's inherent button action.  Makes sense for the kind of MenuMorph that is a wrapper for a single menu-item -- pass it on the the item"
   
    (self findA: MenuItemMorph) ifNotNil: [:aMenuItem | aMenuItem doButtonAction]


And for me it doesn't make sense :).  Even the comment doesn't make sense, because it isn't considering the case where a MenuMorph has multiple MenuItemMorph s.

What should be done in cases like that?  If it's my code, i should be tempted to delete it or maybe to deprecate it.  And I'm really upset because it is a very difficult thing to verify not to break too much other things.  So, what is the "official policy" about this cases? :P  I don't like to leave it as it is...

Cheers,
Guille

_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: Issue 2756: Morphic Cleansing - Remove use of findA:

Stéphane Ducasse

On Aug 6, 2010, at 4:54 AM, Guillermo Polito wrote:

> Hi, removing some findA: message sendings

Yes yes yes!!!
We only need a few of them.

> i've found:
>
> MenuMorph>>doButtonAction
>     "Do the receiver's inherent button action.  Makes sense for the kind of MenuMorph that is a wrapper for a single menu-item -- pass it on the the item"
>    
>     (self findA: MenuItemMorph) ifNotNil: [:aMenuItem | aMenuItem doButtonAction]
>
>
> And for me it doesn't make sense :).  Even the comment doesn't make sense, because it isn't considering the case where a MenuMorph has multiple MenuItemMorph s.
>
> What should be done in cases like that?  If it's my code, i should be tempted to delete it or maybe to deprecate it.  And I'm really upset because it is a very difficult thing to verify not to break too much other things.  So, what is the "official policy" about this cases? :P  I don't like to leave it as it is...

Get one or more scenario where the code behavior normally
and rewrite so that it does not use findA:
What I suggest is that you start with the easy findA: and publish often so that we integrate them in the system
and slowly we learn and fix the ugly.

Stef



>
> Cheers,
> Guille
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Reply | Threaded
Open this post in threaded view
|

Re: Issue 2756: Morphic Cleansing - Remove use of findA:

Guillermo Polito
Hi! I realized that because

Morphic>>findA: aClass
    "Return the first submorph of the receiver that is descended from the given class. Return nil if there is no such submorph. Clients of this code should always check for a nil return value so that the code will be robust if the user takes the morph apart."

    ^self submorphs
        detect: [:p | p isKindOf: aClass]
        ifNone: [nil]


we can simply replace MenuMorph>>doButtonAction by

MenuMorph>>doButtonAction
    "Do the receiver's inherent button action.  A MenuMorph that is a wrapper for a single menu-item -- pass it on the the item. We should enhance or think what to do with a MenuMorph with multiple items"

    self menuItems notEmpty ifTrue:[ self menuItems first doActionButton ]


I'll look at that a few minutes to give a little fix :).

On Fri, Aug 6, 2010 at 6:28 PM, Stéphane Ducasse <[hidden email]> wrote:

On Aug 6, 2010, at 4:54 AM, Guillermo Polito wrote:

> Hi, removing some findA: message sendings

Yes yes yes!!!
We only need a few of them.

> i've found:
>
> MenuMorph>>doButtonAction
>     "Do the receiver's inherent button action.  Makes sense for the kind of MenuMorph that is a wrapper for a single menu-item -- pass it on the the item"
>
>     (self findA: MenuItemMorph) ifNotNil: [:aMenuItem | aMenuItem doButtonAction]
>
>
> And for me it doesn't make sense :).  Even the comment doesn't make sense, because it isn't considering the case where a MenuMorph has multiple MenuItemMorph s.
>
> What should be done in cases like that?  If it's my code, i should be tempted to delete it or maybe to deprecate it.  And I'm really upset because it is a very difficult thing to verify not to break too much other things.  So, what is the "official policy" about this cases? :P  I don't like to leave it as it is...

Get one or more scenario where the code behavior normally
and rewrite so that it does not use findA:
What I suggest is that you start with the easy findA: and publish often so that we integrate them in the system
and slowly we learn and fix the ugly.

Stef



>
> Cheers,
> Guille
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project


_______________________________________________
Pharo-project mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project