jueves, 8 de mayo de 2008

Ventajas del código autodocumentado

Este año hay un grupo muy activo en POO lo cual es muy bueno porque preguntan y hacen cosas más allá de lo que comúnmente sucede en esta materia.
Por ejemplo, el otro día en la lista de POO un alumno preguntó como hacer una búsqueda de algún String particular en los métodos. Pareciera que en VisualWorks no hay manera de hacerlo directamente usando las herramientas de desarrollo que ofrece, pero como esto es Smalltalk no cuesta nada escribir un conjunto de colaboraciones para hacerlo. Así se los propuse y, debido a que es un grupo muy interesante, generaron una solución.
La solución que generaron fue:

| resultado aBuscar bloque |

resultado := OrderedCollection new.
aBuscar := (Dialog request: 'Texto a buscar:' initialAnswer: '' windowLabel: 'Buscar' for: nil).
bloque := [:class :selector :aString | | method |
method := class compiledMethodAt: selector.
( method allLiterals anySatisfy: [:lit | lit isString and: [aString match: lit]])
ifTrue: [ resultado add: method definition]
].
Object allSubclasses do: [:class | class selectors do: [:sel | bloque value: class value: sel value: aBuscar ]].

RefactoringBrowser
openListBrowserOn: resultado
label: 'Resultados de búsqueda'
initialSelection: nil

Me parece interesante analizar los errores que posee dicho fragmento de código y como se puede escribir de otra manera para que sea más simple de entender. Como errores podríamos enumerar:
1) No tiene en cuenta el caso de presionar Cancel cuando se pregunta por el string. En dicho caso buscará el String ''
2) No tiene en cuenta aquellas clases que no tienen superclase que no sea Object
3) No busca el string en Object puesto que hace Object allSubclasses en vez de Object withAllSubclasses

Respecto a como está escrito, los nombre de las variables que usaron no están muy buenos. Dichos nombres deberían ser más explicativos, por ejemplo "resultado" no dice mucho sobre el objeto que referencia.
El objeto "bloque", también es un poco confuso, no solo por el nombre sino por como termina participando en las colaboraciones. Me imagino que la intención fue refactorizar el código puesto que de estar escribiendo esto en una clase ese "bloque" estaría representado por un método. Ahora, de ser así, seguramente ese método no se activaría al enviar el mensaje #bloque, lo que demuestra que "bloque" no es un buen nombre para dicho objeto.
Mi propuesta para resolver este problema es:

| methods stringToLookFor |

methods := OrderedCollection new.
stringToLookFor := Dialog request: 'Texto a buscar:' initialAnswer: '' windowLabel: 'Buscar' for: nil.
stringToLookFor isEmpty ifTrue: [^self].

Class rootsOfTheWorld do: [ :aRootClass |
aRootClass withAllSubclasses do: [ :aClass | | methodContainingStringToLookFor |
methodContainingStringToLookFor := aClass methodDictionary values select: [ :aMethod |
aMethod allLiterals anySatisfy: [ :aLiteral | aLiteral isString and: [ stringToLookFor match: aLiteral ]]].
methods addAll: methodContainingStringToLookFor ]].

RefactoringBrowser
openListBrowserOn: (methods collect: [ :aMethod | aMethod definition ])
label: 'Resultados de búsqueda'
initialSelection: nil

Esta solución resuelve los errores del script anterior. Creo que es un poco más fácil de leer, pero el hecho de tener tantos bloques anidados complica bastante su lectura. Para hacer un poco más entendible el script, utilizo la variable methodContainingStringToLookFor para explicar qué hace el conjunto de colaboraciones "aClass methodDictionary values select: ...". De esta manera no es necesario leer toda las colaboraciones para entender qué resulta al evaluarlas.
Sin embargo, la mejor solución sería modelar esto en una clase (olvidarnos del script...), puesto que al hacerlo se podría refactorizar bastante y hacer que el código quede más "autodocumentado".
De hacerlo, obtendríamos algo así:
...>>methodsContaining: aString

^Class rootsOfTheWorld inject: OrderedCollection new into: [ :methods :aRootClass |
methods addAll: (self methodsContaining: aString inAll: aRootClass withAllSubclasses).
methods]

...>>methodsContaining: aString inAll: aCollectionOfClasses

^aCollectionOfClasses inject: OrderedCollection new into: [:methods :aClass |
methods addAll: (self methodsContaining: aString in: aClass) ]

..>>methodsContaining: aString in: aClass

"Lamentablemente VisualWorks no define le mensaje #methods en Behavior, por eso trabajo con el methodDictionary. Esto rompe el encapsulamiento pero es una solución de compromiso para no modificar Behavior"
^aClass methodDictionary values select: [ :aMethod | self does: aMethod includes: aString ]

...>>does: aMethod includes: aString

^aMethod allLiterals anySatisfy: [ :aLiteral | aLiteral isString and: [ aString match: aLiteral ]].

Creo que así queda mucho más lindo y fácil de entender. Fijensé como desaparece la necesidad de definir la variable methodContainingStringToLookFor debido a que fue reemplazada por el envío de un mensaje, #methodsContaining:inAll: , que a mi gusto además transmite mejor la intención de qué están haciendo los objetos.
Fijensé también que los métodos #methodsContaining:in: y #does:includes: podrían haber sido implementados en Behavior y CompiledCode respectivamente, de tal manera que quedasen como #methodsContainingString: e #includesString:
Me parece interesante este caso como ejemplo de cómo se puede mejorar la legibilidad del código y de cómo así y todo en algo tan sencillo pueden haber varios errores.

1 comentario:

leynar dijo...

Hola soy un estudiante de la Universidad de Granada, que llegó a su blog a través de Google, buscando como poder hacer entradas desde teclado en SmallTalk. Estoy usando el entorno Squeak, pero no se porque pero la clase Dialog no me aparece en mi imagen...

¿Sabe si hay alguna otra forma?

Un saludo y gracias