On 18 June 2013 11:20, Goubier Thierry <[email protected]> wrote: > 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: > +1
> 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 > -- Best regards, Igor Stasenko.
