fsprojects / FSharpLint

Lint tool for F#
https://fsprojects.github.io/FSharpLint/
MIT License
304 stars 73 forks source link

Should not suggest composition over lambdas like `fun x -> -sin x` #372

Open breki opened 4 years ago

breki commented 4 years ago

Description

FSharpLint suggests I should replace lambda expression like fun x -> -sin x with composition, which makes a much less legible code (like sin >> ((*) -1.) or sin >> (~-)).

Repro steps

Define a lambda like

let sampleEnergy= (fun x -> -sin x)

Expected behavior

FSharpLint should accept the existing lambda expression as it is.

abelbraaksma commented 4 years ago

While a good point for the case you mentioned, there are many cases where composition creates better readable code, like:

// this
let yourName = fun() -> getInput() |> fun x sprintf "name: %s" x
// vs:
let yourName = getInput >> sprintf "name: %s" 

And many variants of that. Perhaps the rule could be tailored for cases that involve operators. Though personally, I don't mind seeing (op) in code.

BTW, your rewrite is not synonymous with the original. You wrote sin >> (~-), but that should probably be (~-) >> sin, if it is to match fun x -> -sin x (just saying in case it's an error in your original code, the example is otherwise clear ;).

dsyme commented 4 years ago

My recommendation is to find criteria to reduce the recommendation of >> composition.

For example never for operators.

Perhaps some other cases too. See my talk "F# Code I Love" for exposition of some of this.

breki commented 4 years ago

BTW, your rewrite is not synonymous with the original. You wrote sin >> (~-), but that should probably be (~-) >> sin, if it is to match fun x -> -sin x (just saying in case it's an error in your original code, the example is otherwise clear ;).

Yeah, sorry, it was suggested by someone on Twitter and I didn't really check it before posting it as an example here, my bad :-(. I agree about the usefulness of composition, it's just that I don't feel the code is more readable in this particular case of a unary operator being applied.

kerams commented 4 years ago

@abelbraaksma

BTW, your rewrite is not synonymous with the original. You wrote sin >> (~-), but that should probably be (~-) >> sin, if it is to match fun x -> -sin x

No, yours isn't ;). First apply sin, then negate. It's just that it makes no difference in the case of sine since sin -x = -sin x