Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider using other name for Option.filter #263

Open
PawelStadnicki opened this issue Apr 18, 2024 · 3 comments
Open

Consider using other name for Option.filter #263

PawelStadnicki opened this issue Apr 18, 2024 · 3 comments

Comments

@PawelStadnicki
Copy link

Is your feature request related to a problem? Please describe.

Option.filter meaning is not clear as it typically resides in collection modules and here we only operate on a single value. And when I understood it what it does and started using it , I had the same problem while presenting the code to my peers. They don't understand it too at a first glance.

Additionally filter even with collections provides confusion, as what indeed it does is filtering OUT item(s)

Describe the solution you'd like
A better word for that function. First in mind is 'accept' but it is very subjective too.

// proceed with processing option only if something is true
amount
|> Option.accept ((=) 1000)
|> Option.either (fun _ -> "bingo") (fun _ -> "not good")

Describe alternatives you've considered

module Option =
  let accept = Option.filter

but I thing adding another name for 3rd party lib is not perfect

Additional context
Downsides of the proposition: When filter is replaced it is a breaking change, when it is left with another name for the same purpose it brings confusion.

@TheAngryByrd
Copy link
Collaborator

👋 Yeah some names aren't straight forward and are chosen because of their resemblance to their list counterparts.

Additionally filter even with collections provides confusion, as what indeed it does is filtering OUT item(s)

Actually filter does filter, but it filters for the predicate value. Think of it like filtering for the thing you want. A coffee filter, filters for coffee (the liquid) and filters out anything else (the grounds).

Personally I find filter weird also and would prefer filterFor and filterOut. Would be more clear when coffee filtering what you're looking for. (filterFor (fun x -> x.IsLiquid)/filterOut(fun x -> x.AreGrounds)


That being said, I won't remove filter as that kind of breaking change is not worth it. Also, Option.filter is already part are F# core, so this really wouldn't be solved here. You're still going to need your own alias module without this library.

As far as an alias, where does pop up and people familiar with LINQ/SQL would be more comfortable. I would be open to a PR adding where in addition all the places filter current exists.

@PawelStadnicki
Copy link
Author

Thank you for a very good clarification. I agree with all of that, and indeed Option.where seems to be a better alternative. These filterFor, filterOut also sound interesting. Let me play with it in the project and then I will prepare a PR.

@bartelink
Copy link
Contributor

bartelink commented Apr 19, 2024

TaskSeq has a where for every filter, aligning with FSharp's general stance (even though for me it's a bit of a wart, and not an overall positive to have filter and where being identical given such redundant aliases are not present in anything else in F#)

Though the coffee rationale finally gives me a way to make peace with the filter functions name, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants