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

add Tree.findDepthFirst and Tree.findBreadthFirst and Tree.filter #1541

Open
wants to merge 1 commit into
base: 3.0.0
Choose a base branch
from
Open

Conversation

thekemlin
Copy link

Add some utility functions to Tree module.

@thekemlin
Copy link
Author

Hi @gcanti !
I put lots of effort into the PR I submitted and I would really appreciate it if you can take a look and give me some suggestions on the code if you have any. Thanks!

@kalda341
Copy link

kalda341 commented Aug 22, 2021

This would be really helpful for me too.

One thing to consider would be whether the find and filter should be operating on the tree itself, or the value contained.
I think your approach (the tree itself) is the better one as it is more flexible (and similar to operations I've built). Maybe it's worth implementing both (findNodeDepthFirst, findDepthFirst, or similar naming convention)?

When it comes to filter (which is really useful to me), it would probably be worth having both a depth first and breadth first option if we're operating on the tree, as when filtering an element your predicate may care about how many filtered children remain, for example.

What do you think? I'm happy to help get this across the line.

@thekemlin
Copy link
Author

Maybe it's worth implementing both (findNodeDepthFirst, findDepthFirst, or similar naming convention)?

Yes, It's valuable.

When it comes to filter (which is really useful to me), it would probably be worth having both a depth first and breadth first option.

I am confused.
Are you mean:
When filter on a tree, there should be two ways to traverse the tree:

  1. Apply the predicate to the forest of the node, and then the node's own value.
  2. Apply the predicate to the node's own value, if the result is true, then filter the forest of the node recursively. (current implementation)

@kalda341
Copy link

@polymona Yes, that's what I mean :)

@thekemlin
Copy link
Author

thekemlin commented Aug 26, 2021

@gcanti What do you think? Is it worth to add those functions to fp-ts?

@gcanti gcanti added this to the 3.0.0 milestone Apr 21, 2022
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.

None yet

3 participants