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

FullBinaryTreeOps methods for collecting labels #15

Closed
wants to merge 1 commit into from

Conversation

jhoon-stripe
Copy link
Contributor

When writing tests on binary trees, I've occasionally wanted to be able to obtain a list of the trees' keys, to aid in constructing sample data and models.

This PR adds four methods for collecting labels from full binary trees:

  • collectLabels
  • collectLabelsF
  • collectLeafLabels
  • collectLeafLabelsF

These methods return sets, as we shouldn't have any opinion on the ordering of labels in FullBinaryTreeOps (ordered traversals should be handled in Brushfire's TreeTraversals[0]).

cc @erik-stripe @kelley-stripe @oscar-stripe @rob-stripe @thomas-stripe

[0] https://github.com/stripe/brushfire/blob/master/brushfire-tree/src/main/scala/com/stripe/brushfire/TreeTraversal.scala

Copy link
Contributor

@thomas-stripe thomas-stripe left a comment

Choose a reason for hiding this comment

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

Nice! Yeah, these types are pretty spartan. It'd be nice to really try to make all this stuff more user friendly. Glad to see it getting some love :)

def collectLeafLabelsF[A](node: Node, f: LL => A): Set[A] =
foldNode(node)({case (lc, rc, _) => collectLeafLabelsF(lc, f) | collectLeafLabelsF(rc, f)}, ll => Set(f(ll)))

def collectLabels(node: Node): Set[Label] = collectLabelsF(node, identity)
Copy link
Contributor

Choose a reason for hiding this comment

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

This method could go on TreeOps instead, since it's general enough.

@@ -21,6 +21,17 @@ trait FullBinaryTreeOps[T, BL, LL] extends TreeOps[T, Either[BL, LL]] {

def children(node: Node): Iterable[Node] =
foldNode(node)({ case (lc, rc, _) => lc :: rc :: Nil }, _ => Nil)

def collectLabelsF[A](node: Node, f: Label => A): Set[A] =
Copy link
Contributor

@thomas-stripe thomas-stripe Nov 14, 2016

Choose a reason for hiding this comment

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

If you wanted to, you could implement this w/ reduce:

reduce(node) { case (label, childLabels) => childLabels.foldLeft(Set(label))(_ ++ _) }

Though I'd say there is a case to be made that we should have a FullBinaryTreeOps-specific version of reduce that takes 2 functions, since we shouldn't have to deal with the Iterable childLabels (and it'd make implementing collectLeafLabels in terms of reduce nicer):

def reduceFullBinaryTree[A](node: Node)(f: LL => A)(g: (BL, A, A) => A): A = ???

wdyt?

@travisbrown-stripe
Copy link
Contributor

Included in #19.

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.

4 participants