Hi Camille,
It's not bad as things goes, because it does look a bit like
non-smalltalk code (i.e. the if then else structure) except that the
precise meaning is very much Smalltalk :
if: has a receiver, and there is a not so intuitive relationship between
the bloc parameters and the receiver.
Honestly, as far as code reviews goes, I'll prefer the temp refactoring.
Understandable without having to search for the implementors of
#if:then:else:
Regards,
Thierry
Le 18/06/2013 10:51, Camille Teruel a écrit :
Hello everyone.
I see a lot of code that follows the following pattern:
<a long expression> <aComplexTest>
ifTrue: [ <a long expression> <something> ]
ifFalse: [ <a long expression> <something else> ].
The classic refactoring is to store <a long expression> in a temp to
avoid repetition:
| temp |
temp := <a long expression>.
temp <aComplexTest>
ifTrue: [ temp <something> ]
ifFalse: [ temp <something else> ].
Then <a long expression> is evaluated once and the name of the temp can
carry a meaningful name that helps reading the code.
But this is more verbose than the first version.
But if you add the following method in Object:
if: conditionBlock then: trueBlock else: falseBlock
^ (conditionBlock cull: self)
ifTrue: [ trueBlock cull: value ]
ifFalse: [ falseBlock cull: value ]
You can then rewrite it like that:
<a long expression>
if: [ :obj | obj <aComplexTest> ]
then: [ :obj | <something> ]
else: [ :obj | <something else> ]
The names of the block args can give a meaningful name depending on the
result of the test (something like ... if: #notEmpty then: [ :collection
| ... ] else: [ :emptyCollection | ... ]). Using cull: for the three
block also enable a good flexibility: you can omit the arg when irrelevant.
Likewise, we could also have Object>>#while:do:
while: conditionBlock do: actionBlock
^ [ conditionBlock cull: self ] whileTrue: [ actionBlock cull: self ]
What do you think?
Camille
--
Thierry Goubier
CEA list
Laboratoire des Fondations des Systèmes Temps Réel Embarqués
91191 Gif sur Yvette Cedex
France
Phone/Fax: +33 (0) 1 69 08 32 92 / 83 95
