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

Use List.exists for contains #1518

Merged
merged 3 commits into from
Sep 11, 2016
Merged

Use List.exists for contains #1518

merged 3 commits into from
Sep 11, 2016

Conversation

forki
Copy link
Contributor

@forki forki commented Sep 5, 2016

No description provided.

match l with
| [] -> false
| x'::t -> f x x' || contains f x t
let inline contains f x l = List.exists (f x) l
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@forki
This API look like exists rather than contains. Is it not misnamed? Perhaps we should name it exists and fix up the places where we use it by the wrong name.

exists: https://msdn.microsoft.com/en-us/visualfsharpdocs/conceptual/list.exists%5b't%5d-function-%5bfsharp%5d
contains: https://msdn.microsoft.com/en-us/visualfsharpdocs/conceptual/list.contains%5b't%5d-function-%5bfsharp%5d

Kevin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mhm after thinking about it. All the other listset functions have the first parameter f which is the projection. In this sense it makes sense to revert my last commit and keep it named contains.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The real question is: can we use a different data structure!?

It needs to be a set that allows to pass in own equality semantics.

@KevinRansom
Copy link
Member

Looks good to me,

thanks for this contribution

Kevin

@KevinRansom KevinRansom reopened this Sep 11, 2016
@KevinRansom KevinRansom merged commit e7d09b2 into dotnet:master Sep 11, 2016
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

Successfully merging this pull request may close these issues.

3 participants