jueves, 9 de julio de 2009

viernes, 3 de julio de 2009

Respuesta: ¿Cómo mejorar esta clase?

El otro día postee una pregunta sobre cómo mejorar la clase MouseOverHandler de Squeak/Pharo... no hubo ningún comentario lo que creo que significa que nadie se animó a hacerlo! (o claro, nadie leyó el post...). En fin, creo que nadie se animó a hacerlo porque realmente no es fácil y sólo ver el código desalienta a cualquiera, pero bueno, yo lo hice hace una para de semanas atrás así que les comento cómo.
Primero, ¿cuál es el problema con esta implementación? El principal es que el método #processMouseOver: es muy difícil de entender. Es muy largo y complicado, así que hay que empezar por tratar de entender que hace y mejorarlo. Para ello vamos a utilizar el refactoring de extract method para reemplazar colaboraciones por mensajes que revelen su intención. Lo interesante es que el método está comentado justamente en las secciones que vamos a extraer, lo cual muestra como a veces en vez de comentar código simplemente hay que escribirlo con intention revealing. Veamos como queda.
Reemplacemos este código:

hand := anEvent hand.
leftMorphs := mouseOverMorphs asIdentitySet.
"Assume some coherence for the number of objects in over list"
overMorphs := WriteStream on: (Array new: leftMorphs size).
enteredMorphs := WriteStream on: #().

Por:
self initializeProcessMouseOver.

Luego reemplacemos:
"Now go looking for eventual mouse overs"
hand handleEvent: anEvent asMouseOver.
"Get out early if there's no change"
(leftMorphs isEmpty and: [enteredMorphs position = 0])
ifTrue: [^leftMorphs := enteredMorphs := overMorphs := nil].
focus := hand mouseFocus.

Por:
self handleAsMouseOver: anEvent.

También reemplacemos: (no se preocupen en leer todo el código, simplemente tomen un dimensión de qué estamos reemplazando y por quién)
"Send #mouseLeave as appropriate"
evt := anEvent asMouseLeave.
"Keep the order of the left morphs by recreating it from the mouseOverMorphs"
leftMorphs size > 1
ifTrue: [leftMorphs := mouseOverMorphs select: [:m | leftMorphs includes: m]].
leftMorphs do:
[:m |
(m == focus or: [m hasOwner: focus])
ifTrue:
[localEvt := evt transformedBy: (m transformedFrom: hand).
m handleEvent: localEvt]
ifFalse: [overMorphs nextPut: m]].

Por:
self handleAsMouseLeave: anEvent.

También reemplacemos:
"Send #mouseEnter as appropriate"
evt := anEvent asMouseEnter.
enteredMorphs ifNil:
["inform: was called in handleEvent:"

^leftMorphs := enteredMorphs := overMorphs := nil].
enteredMorphs := enteredMorphs contents.
enteredMorphs reverseDo:
[:m |
(m == focus or: [m hasOwner: focus])
ifTrue:
[localEvt := evt transformedBy: (m transformedFrom: hand).
m handleEvent: localEvt]].

Por:
self handleAsMouseEnter: anEvent.

Y por último:
"And remember the over list"
overMorphs ifNil:
["inform: was called in handleEvent:"

^leftMorphs := enteredMorphs := overMorphs := nil].
mouseOverMorphs := overMorphs contents.
leftMorphs := enteredMorphs := overMorphs := nil

Por:
self rememberOverList

Bueno, no se que les parece (o si quedó claro como lo escribí), pero simplemente hay que agarrar cada conjunto de colaboraciones que tiene un comentario sobre que hacen y extraerlas a otro método. En conclusión con algunos cambios más el método #processMouseOver: quedá así:

processMouseOver: anEvent

self initializeProcessMouseOver.
self handleAsMouseOver: anEvent.
self hasLeftMorphsChanged ifTrue: [
self handleAsMouseLeave: anEvent.
self handleAsMouseEnter: anEvent.
self rememberOverList ].

self initializeTrackedMorphs

¿Se entiende mejor qué hace no?. Fijensé como un cambio tan sencillo, simplemente usando extract method, mejora el diseño considerablemente. Por supuesto que hay que seguir utilizando esta técnica para cada uno de los nuevos métodos porque siguen siendo muy complejos y al hacerlo se puede ver que hay código repetido, de hecho el mismo refactoring browser al realizar la extracción lo sugiere. No voy a mostrar todo este proceso porque es similar al anterior y no quiero hacer el post muy pesado (más de lo que es!).
Por último, también realicé otros cambios como por ejemplo no usar nil para inicializar las variables de instancia que utiliza para realizar el procesamiento, sino inicializarlas con colecciones vacías de tal manera que nunca se produzca un MNU.
En conclusión, el código quedó así:

Object subclass: #MouseOverHandler
instanceVariableNames: 'mouseOverMorphs enteredMorphs overMorphs leftMorphs'
classVariableNames: ''
poolDictionaries: ''
category: 'Morphic-Events'!
-------------------------------------------------------------------------------
processMouseOver: anEvent

self initializeProcessMouseOver.
self handleAsMouseOver: anEvent.
self hasLeftMorphsChanged ifTrue: [
self handleAsMouseLeave: anEvent.
self handleAsMouseEnter: anEvent.
self rememberOverList ].

self initializeTrackedMorphs
-------------------------------------------------------------------------------
initializeProcessMouseOver

leftMorphs := mouseOverMorphs asIdentitySet.
overMorphs := WriteStream on: (Array new: leftMorphs size).
enteredMorphs := WriteStream on: #()! !
-------------------------------------------------------------------------------
handleAsMouseEnter: anEvent

| asMouseEnterEvent |

asMouseEnterEvent := anEvent asMouseEnter.
enteredMorphs := enteredMorphs contents.
enteredMorphs reverseDo: [ :anEnteredMorph |
self inform: asMouseEnterEvent to: anEnteredMorph originatedFrom: anEvent ifNotFocusedDo: [] ]
-------------------------------------------------------------------------------
handleAsMouseLeave: anEvent

self keepLeftMorphsOrder.
self informMouseLeaveToLeftMorphsUsing: anEvent
-------------------------------------------------------------------------------
handleAsMouseOver: anEvent

anEvent hand handleEvent: anEvent asMouseOver.
-------------------------------------------------------------------------------
noticeMouseOver: aMorph event: anEvent
"Remember that the mouse is currently over some morph"

leftMorphs remove: aMorph ifAbsent: [ enteredMorphs nextPut: aMorph ].
overMorphs nextPut: aMorph.
-------------------------------------------------------------------------------
initialize
mouseOverMorphs := #().

self initializeTrackedMorphs
-------------------------------------------------------------------------------
initializeTrackedMorphs

leftMorphs := OrderedCollection new.
overMorphs := WriteStream on: #().
enteredMorphs := WriteStream on: #().
-------------------------------------------------------------------------------
hasLeftMorphsChanged

^(leftMorphs isEmpty and: [ enteredMorphs position = 0 ]) not
-------------------------------------------------------------------------------
inform: evt to: aLeftMorph originatedFrom: anEvent ifNotFocusedDo: aBlock

^ (self is: anEvent withFocusOver: aLeftMorph)
ifTrue: [ self transform: evt from: anEvent andSendTo: aLeftMorph ]
ifFalse: aBlock
-------------------------------------------------------------------------------
informMouseLeaveToLeftMorphsUsing: anEvent

| asMouseLeaveEvent |

asMouseLeaveEvent := anEvent asMouseLeave.

leftMorphs do: [ :aLeftMorph |
self inform: asMouseLeaveEvent to: aLeftMorph originatedFrom: anEvent ifNotFocusedDo: [ overMorphs nextPut: aLeftMorph ] ]
-------------------------------------------------------------------------------
is: anEvent withFocusOver: aMorph

| focusedMorph |

focusedMorph := anEvent hand mouseFocus.
^ aMorph = focusedMorph or: [ aMorph hasOwner: focusedMorph ]
-------------------------------------------------------------------------------
keepLeftMorphsOrder

leftMorphs size > 1 ifTrue: [ leftMorphs := mouseOverMorphs intersection: leftMorphs ]
-------------------------------------------------------------------------------
rememberOverList

mouseOverMorphs := overMorphs contents.
-------------------------------------------------------------------------------
transform: anEvent from: originalEvent andSendTo: aMorph

| transformedEvent |

transformedEvent := anEvent transformedBy: (aMorph transformedFrom: originalEvent hand).
^ aMorph handleEvent: transformedEvent
-------------------------------------------------------------------------------
¿Mucho más claro no?

jueves, 2 de julio de 2009

miércoles, 1 de julio de 2009

Quiz: ¿Cómo mejorar esta clase?

Que tal,
les dejo un problema para pensar, no es muy largo ni difícil, con 10 minutos imagino que pueden encontrar la manera de resolverlo. El problema es el siguiente: hay que mejorar el código de la clase MouseOverHandler que se encuentra en Squeak o Pharo, en particular el método #processMouseOver: anEvent.
El motivo por el cual seleccioné este problema se debe a que últimamente en la imagen de Pharo en condiciones aún no encontradas, el método #noticeMouseOver: aMorph event: anEvent envía el mensaje #includes: a nil.
Lo interesante del problema es:
1) Entender como funcionan las instancias de esta clase
2) Encontrar el motivo de por qué funciona incorrectamente el mensaje #noticeMouseOver: aMorph event: anEvent
3) Pensar cómo se puede generar información que confirme lo encontrado en el punto 2
4) El objetivo de este post, mejorar el diseño de esta clase.
Acá está el código por si no tienen ganas de abrir un Squeak/Pharo.
---------------------------
Object subclass: #MouseOverHandler
instanceVariableNames: 'mouseOverMorphs enteredMorphs overMorphs leftMorphs'
classVariableNames: ''
poolDictionaries: ''
category: 'Morphic-Events'
---------------------------
initialize
super initialize.
mouseOverMorphs := #().
---------------------------
processMouseOver: anEvent
"Re-establish the z-order for all morphs wrt the given event"

| hand localEvt focus evt |
hand := anEvent hand.
leftMorphs := mouseOverMorphs asIdentitySet.
"Assume some coherence for the number of objects in over list"
overMorphs := WriteStream on: (Array new: leftMorphs size).
enteredMorphs := WriteStream on: #().
"Now go looking for eventual mouse overs"
hand handleEvent: anEvent asMouseOver.
"Get out early if there's no change"
(leftMorphs isEmpty and: [enteredMorphs position = 0])
ifTrue: [^leftMorphs := enteredMorphs := overMorphs := nil].
focus := hand mouseFocus.
"Send #mouseLeave as appropriate"
evt := anEvent asMouseLeave.
"Keep the order of the left morphs by recreating it from the mouseOverMorphs"
leftMorphs size > 1
ifTrue: [leftMorphs := mouseOverMorphs select: [:m | leftMorphs includes: m]].
leftMorphs do:
[:m |
(m == focus or: [m hasOwner: focus])
ifTrue:
[localEvt := evt transformedBy: (m transformedFrom: hand).
m handleEvent: localEvt]
ifFalse: [overMorphs nextPut: m]].
"Send #mouseEnter as appropriate"
evt := anEvent asMouseEnter.
enteredMorphs ifNil:
["inform: was called in handleEvent:"

^leftMorphs := enteredMorphs := overMorphs := nil].
enteredMorphs := enteredMorphs contents.
enteredMorphs reverseDo:
[:m |
(m == focus or: [m hasOwner: focus])
ifTrue:
[localEvt := evt transformedBy: (m transformedFrom: hand).
m handleEvent: localEvt]].
"And remember the over list"
overMorphs ifNil:
["inform: was called in handleEvent:"

^leftMorphs := enteredMorphs := overMorphs := nil].
mouseOverMorphs := overMorphs contents.
leftMorphs := enteredMorphs := overMorphs := nil
---------------------------
noticeMouseOver: aMorph event: anEvent
"Remember that the mouse is currently over some morph"
(leftMorphs includes: aMorph)
ifTrue:[leftMorphs remove: aMorph]
ifFalse:[enteredMorphs nextPut: aMorph].
overMorphs nextPut: aMorph.

Alguien está necesitado de dinero...

Bueno, quién no? pero miren esto:
http://cgi.ebay.com/ws/eBayISAPI.dll?ViewItem&item=180375524276
Inicialmente me hace pensar en dos cosas:
1) Ya nadie se gana la vida escribiendo libros (o por lo menos libros técnicos)
2) Kent Beck no es muy bueno haciendo negocios :-)
El resto de las posibilidad lo dejo par el ideario colectivo :-)