Skip to content

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

Open
jituanlin wants to merge 1 commit into
gcanti:3.0.0from
jituanlin:3.0.0
Open

add Tree.findDepthFirst and Tree.findBreadthFirst and Tree.filter#1541
jituanlin wants to merge 1 commit into
gcanti:3.0.0from
jituanlin:3.0.0

Conversation

@jituanlin

Copy link
Copy Markdown

Add some utility functions to Tree module.

@jituanlin

Copy link
Copy Markdown
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

kalda341 commented Aug 22, 2021

Copy link
Copy Markdown

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.

@jituanlin

Copy link
Copy Markdown
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
Copy Markdown

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

@jituanlin

jituanlin commented Aug 26, 2021

Copy link
Copy Markdown
Author

@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.

3 participants