Rationale behind the "ifTrue:/ifFalse: returns instead of and:/or:'s" rule
Hi, there is a rule that suggests to use and/or boolean operations instead of multiple returns. For example it suggests agains using: isTranslucentButNotTransparent backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false Instead you should use: isTranslucentButNotTransparent ^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ] And at least a few developers think that the suggested implementation is more complicated to comprehend. What is the reasoning behind the rule? Does the suggested version run faster (i.e. is optimized ing some way?). Cheers. Uko
Hi Yuriy, to me the first form is a list of simple rules. The second one is a complex expression You made an effort to keep the list format for the second expression, while violating traditional block formatting which would be ^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ] I don't think that runtime optimization count at all in such choice. 2017-04-07 10:33 GMT+02:00 Yuriy Tymchuk <yuriy.tymchuk@me.com>:
Hi, there is a rule that suggests to use and/or boolean operations instead of multiple returns.
For example it suggests agains using:
isTranslucentButNotTransparent
backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false
Instead you should use:
isTranslucentButNotTransparent
^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ]
And at least a few developers think that the suggested implementation is more complicated to comprehend.
What is the reasoning behind the rule? Does the suggested version run faster (i.e. is optimized ing some way?).
Cheers. Uko
2017-04-07 11:03 GMT+02:00 Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com>:
Hi Yuriy, to me the first form is a list of simple rules. The second one is a complex expression
You made an effort to keep the list format for the second expression, while violating traditional block formatting which would be
^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ]
Sorry, premature post caused by missuse of tab key, traditional format could be something like: isTranslucentButNotTransparent ^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ] Personnally, I like this indentation better. It's still preserving the list of simple rules, and indicate a hierarchy among the rules But indentations sounds overkill and won't scale very high. To me the first form is nearer the pattern matching style of a functional language.
I don't think that runtime optimization count at all in such choice.
2017-04-07 10:33 GMT+02:00 Yuriy Tymchuk <yuriy.tymchuk@me.com>:
Hi, there is a rule that suggests to use and/or boolean operations instead of multiple returns.
For example it suggests agains using:
isTranslucentButNotTransparent
backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false
Instead you should use:
isTranslucentButNotTransparent
^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ]
And at least a few developers think that the suggested implementation is more complicated to comprehend.
What is the reasoning behind the rule? Does the suggested version run faster (i.e. is optimized ing some way?).
Cheers. Uko
On 7 Apr 2017, at 10:33, Yuriy Tymchuk <yuriy.tymchuk@me.com> wrote:
Hi, there is a rule that suggests to use and/or boolean operations instead of multiple returns.
For example it suggests agains using:
isTranslucentButNotTransparent
backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false
Instead you should use:
isTranslucentButNotTransparent
^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ]
And at least a few developers think that the suggested implementation is more complicated to comprehend.
I really do not like the complex boolean expressions⦠my brain can understand the âcheck and returnâ guards extremely fast: backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false while for the nesting everything in one huge or: /and: expression I have to think. I always feel that replacing the âguardâ style with complex nested booleans makes the code worse. I really would like to remove the rule. Marcus
2017-04-07 11:23 GMT+02:00 marcus.denker@inrira.fr <marcus.denker@inria.fr>:
I really do not like the complex boolean expressions⦠my brain can understand the âcheck and returnâ guards extremely fast:
backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false
while for the nesting everything in one huge or: /and: expression I have to think. I always feel that replacing the âguardâ style with complex nested booleans makes the code worse.
I really would like to remove the rule.
Me too
On 7 Apr 2017, at 11:23, marcus.denker@inrira.fr <marcus.denker@inria.fr> wrote:
On 7 Apr 2017, at 10:33, Yuriy Tymchuk <yuriy.tymchuk@me.com> wrote:
Hi, there is a rule that suggests to use and/or boolean operations instead of multiple returns.
For example it suggests agains using:
isTranslucentButNotTransparent
backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false
Instead you should use:
isTranslucentButNotTransparent
^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ]
And at least a few developers think that the suggested implementation is more complicated to comprehend.
I really do not like the complex boolean expressions⦠my brain can understand the âcheck and returnâ guards extremely fast:
backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false
while for the nesting everything in one huge or: /and: expression I have to think. I always feel that replacing the âguardâ style with complex nested booleans makes the code worse.
I really would like to remove the rule.
+42
Marcus
On 7 Apr 2017, at 12:01, Esteban Lorenzano <estebanlm@gmail.com> wrote:
On 7 Apr 2017, at 11:23, marcus.denker@inrira.fr <mailto:marcus.denker@inrira.fr> <marcus.denker@inria.fr <mailto:marcus.denker@inria.fr>> wrote:
On 7 Apr 2017, at 10:33, Yuriy Tymchuk <yuriy.tymchuk@me.com <mailto:yuriy.tymchuk@me.com>> wrote:
Hi, there is a rule that suggests to use and/or boolean operations instead of multiple returns.
For example it suggests agains using:
isTranslucentButNotTransparent
backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false
Instead you should use:
isTranslucentButNotTransparent
^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ]
And at least a few developers think that the suggested implementation is more complicated to comprehend.
I really do not like the complex boolean expressions⦠my brain can understand the âcheck and returnâ guards extremely fast:
backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false
while for the nesting everything in one huge or: /and: expression I have to think. I always feel that replacing the âguardâ style with complex nested booleans makes the code worse.
I really would like to remove the rule.
+42
1) itâs not only about removing. Maybe we should detect complicated boolean expressions that return something and suggest to break them down. 2) I just want to know why someone created such rule :). Because the author of the last update of the initialize method is Marcus :) Uko
Marcus
The Boolean expression is easier to extract methods with an intention revealing name. The "case with returns " it's harder to automatically refactor. And if you get rid of the nils, it became easier to read and refactor. ð On Apr 7, 2017 07:06, "Yuriy Tymchuk" <yuriy.tymchuk@me.com> wrote: On 7 Apr 2017, at 12:01, Esteban Lorenzano <estebanlm@gmail.com> wrote: On 7 Apr 2017, at 11:23, marcus.denker@inrira.fr <marcus.denker@inria.fr> wrote: On 7 Apr 2017, at 10:33, Yuriy Tymchuk <yuriy.tymchuk@me.com> wrote: Hi, there is a rule that suggests to use and/or boolean operations instead of multiple returns. For example it suggests agains using: isTranslucentButNotTransparent backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false Instead you should use: isTranslucentButNotTransparent ^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ] And at least a few developers think that the suggested implementation is more complicated to comprehend. I really do not like the complex boolean expressions⦠my brain can understand the âcheck and returnâ guards extremely fast: backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false while for the nesting everything in one huge or: /and: expression I have to think. I always feel that replacing the âguardâ style with complex nested booleans makes the code worse. I really would like to remove the rule. +42 1) itâs not only about removing. Maybe we should detect complicated boolean expressions that return something and suggest to break them down. 2) I just want to know why someone created such rule :). Because the author of the last update of the initialize method is Marcus :) Uko Marcus
1) itâs not only about removing. Maybe we should detect complicated boolean expressions that return something and suggest to break them down.
No ideaâ¦
2) I just want to know why someone created such rule :). Because the author of the last update of the initialize method is Marcus :)
I did not add it originally. Marcus
2) I just want to know why someone created such rule :). Because the author of the last update of the initialize method is Marcus :)
Probably to stop people from writing code like this: (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false instead of: ^borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] :) John Brant
On 7 Apr 2017, at 10:33, Yuriy Tymchuk <yuriy.tymchuk@me.com> wrote:
Hi, there is a rule that suggests to use and/or boolean operations instead of multiple returns.
For example it suggests agains using:
isTranslucentButNotTransparent
backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false
Instead you should use:
isTranslucentButNotTransparent
^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ]
And at least a few developers think that the suggested implementation is more complicated to comprehend.
What is the reasoning behind the rule? Does the suggested version run faster (i.e. is optimized ing some way?).
I disagree with such rule. Is confusing and unnecessary. The âescape conditionâ is clearer and easier to understand. And methods that are composed like this: a = x1 ifTrue: [ ^ r1 ]. a = x2 ifTrue: [ ^ r2 ]. ⦠etc. are very common because they are (again) legible and can be easily optimised by the JIT. (they are even called âcase methodsâ somewhere I do not remember :P) Esteban
Cheers. Uko
On 7 Apr 2017, at 11:59, Esteban Lorenzano <estebanlm@gmail.com> wrote:
On 7 Apr 2017, at 10:33, Yuriy Tymchuk <yuriy.tymchuk@me.com> wrote:
Hi, there is a rule that suggests to use and/or boolean operations instead of multiple returns.
For example it suggests agains using:
isTranslucentButNotTransparent
backgroundColor ifNil: [ ^ true ]. (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. (borderColor isColor and: [ borderColor isTranslucentButNotTransparent ]) ifTrue: [ ^ true ]. ^ false
Instead you should use:
isTranslucentButNotTransparent
^ backgroundColor isNil or: [ (backgroundColor isColor and: [ backgroundColor isTranslucentButNotTransparent ]) or: [ borderColor isColor and: [ borderColor isTranslucentButNotTransparent ] ] ]
And at least a few developers think that the suggested implementation is more complicated to comprehend.
What is the reasoning behind the rule? Does the suggested version run faster (i.e. is optimized ing some way?).
I disagree with such rule. Is confusing and unnecessary.
The âescape conditionâ is clearer and easier to understand. And methods that are composed like this:
a = x1 ifTrue: [ ^ r1 ]. a = x2 ifTrue: [ ^ r2 ]. ⦠etc.
are very common because they are (again) legible and can be easily optimised by the JIT.
(they are even called âcase methodsâ somewhere I do not remember :P)
of course better way to deal with this is to use double dispatch, but you cannot always do it ;)
Esteban
Cheers. Uko
participants (7)
-
Denis Kudriashov -
Esteban Lorenzano -
Gabriel Cotelli -
John Brant -
marcus.denker＠inrira.fr -
Nicolas Cellier -
Yuriy Tymchuk