Search expression for Code Critic

> Hi,
>
> what would be the correct search expression for SmallLint to find the
> following pattern:
>
> condition
>     ifTrue:[ ^result1 ]
>     ifFalse:[ ^result2 ]
>
> or:
>
> condition
>     ifTrue:[ ^result1 ].
> ^result2
>
> in order to replace it with
>
> ^condition
>     ifTrue:[ result1 ]
>     ifFalse:[ result2 ]
>
> I tested with VisualPart>>topComponent and interestingly the last form
> turned out to be around 10% faster than the first. In certain hot
> spots of an application, this could add up and make a difference.
> Apart from the performance aspect, the last form is also more
> functional style and less intention revealing (the intention being
> that "condition" actually decides upon the return value).
>
> Therefore I thought it would be a good idea to add this pattern to the
> Code Critic set of rules. The search pattern, of course, should allow
> any number of statements prior to result1/2.
>

>  search rule 1:
>
> `@condition
>    ifTrue:[ ^`@result1 ]
>    ifFalse:[ ^`@result2 ]
>
> search rule 2:
>
> `@condition
>    ifTrue:[ ^`@result1 ].
>  ^ `@result2
>
> replace with:
>
> ^ `@condition
>    ifTrue: [ `@result1 ]
>    ifFalse: [ `@result2 ].
>
>
>> Hi,
>>
>> what would be the correct search expression for SmallLint to find the
>> following pattern:
>>
>> condition
>>    ifTrue:[ ^result1 ]
>>    ifFalse:[ ^result2 ]
>>
>> or:
>>
>> condition
>>    ifTrue:[ ^result1 ].
>> ^result2
>>
>> in order to replace it with
>>
>> ^condition
>>    ifTrue:[ result1 ]
>>    ifFalse:[ result2 ]
>>
>> I tested with VisualPart>>topComponent and interestingly the last  
>> form
>> turned out to be around 10% faster than the first. In certain hot
>> spots of an application, this could add up and make a difference.
>> Apart from the performance aspect, the last form is also more
>> functional style and less intention revealing (the intention being
>> that "condition" actually decides upon the return value).
>>
>> Therefore I thought it would be a good idea to add this pattern to  
>> the
>> Code Critic set of rules. The search pattern, of course, should allow
>> any number of statements prior to result1/2.
>>
>
> _______________________________________________
> vwnc mailing list
> [hidden email]
> http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

> Me stupid. I missed the point that names used for matching are  
> arbitrary. Expecting a syntax behind the names confused me. It's  
> really simple. Now this is the complete rule, which also allows none  
> or more statements before the returns:
>
> redundantReturns
> ^self
>    createParseTreeRule: #(
>      '`@condition ifTrue:[ `@.Statements1. ^`@result1 ] ifFalse:
> [ `@.Statements2. ^`@result2 ]'
>      '`@condition ifTrue:[ `@.Statements1. ^`@result1 ]. ^`@result2'
>    )
>    name: (#UnnecessaryReturns << #browser >> 'Unnecessary returns from  
> both ifTrue:ifFalse blocks') asString

> Hi,
>
> what would be the correct search expression for SmallLint to find the
> following pattern:
>
> condition
>     ifTrue:[ ^result1 ]
>     ifFalse:[ ^result2 ]
>
> or:
>
> condition
>     ifTrue:[ ^result1 ].
> ^result2
>
> in order to replace it with
>
> ^condition
>     ifTrue:[ result1 ]
>     ifFalse:[ result2 ]
>
> I tested with VisualPart>>topComponent and interestingly the last form
> turned out to be around 10% faster than the first. In certain hot
> spots of an application, this could add up and make a difference.
> Apart from the performance aspect, the last form is also more
> functional style and less intention revealing (the intention being
> that "condition" actually decides upon the return value).

> Without questioning the style aspect, I wonder how you measured that  
> 10% performance boost. Because in the simplest form of method, like
> ^ boolean
> ifTrue: [ 1 ]
> ifFalse: [ 2 ]
> the bytecodes for its variations (two alternate returns or two  
> returns with a guard clause) are exactly the same:
>  1 <00> push inst 0
>  2 <C1> jump false 5
>  3 <4A> push 1
>  4 <65> return
>  5 <4B> push 2
>  6 <65> return
> for the three variants. The only measurable execution time  
> differences even depend on order of execution after creation of the  
> object understanding this method.
>
> Regards
>  Jan