pharo-project / pharo

Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk.
http://pharo.org
Other
1.19k stars 353 forks source link

Quality rule "Guarding clauses" is not good #7954

Open estebanlm opened 3 years ago

estebanlm commented 3 years ago

Thing is... not always a guarding clause is better than a block in an if, because of readability reasons. For example, the QA will declare this is a QA problem:

rightClicked
    | evt |

    evt := MouseButtonEvent new 
        setType: nil 
        position: widget center 
        which: MouseButtonEvent yellowButton 
        buttons: MouseButtonEvent yellowButton 
        hand: nil 
        stamp: nil.

    (widget handlesMouseDown: evt) ifTrue: [
        widget mouseDown: evt.
        widget mouseUp: evt ]

but in fact, I disagree with that rule, because this is clearly more readable than:

rightClicked
    | evt |

    evt := MouseButtonEvent new 
        setType: nil 
        position: widget center 
        which: MouseButtonEvent yellowButton 
        buttons: MouseButtonEvent yellowButton 
        hand: nil 
        stamp: nil.

    (widget handlesMouseDown: evt) ifFalse: [ ^ self ].
    widget mouseDown: evt.
    widget mouseUp: evt.

notice that I am not against guarding clauses, I like them... is just that you cannot use them all the time. In particular I often call them "escape clauses", because I put them to "escape" a method at the beginning.

MarcusDenker commented 3 years ago

yes, the escape pattern is only readable when it is a the start of a method. (first line or a line after another escape clause)