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

repeat step similar (but superior to) tinkerpop's #118

Merged
merged 52 commits into from Jul 2, 2020

Conversation

mpollmeier
Copy link
Contributor

It's superior to Tinkerpop's repeat because:

  • clearer syntax: tinkerpop uses arbitrary emit/repeat/until combinations which have semantic differences due to their order
  • works for domain-specific traversals as well as generic graph traversals
  • supports DFS and BFS (tp says they use DFS, but actually does use BFS)
  • supports arbitrary number of iterations (tp stalls)

from the scaladoc:
Repeat the given traversal

The @param behaviourBuilder allows you to configure 1) when the repeat step will end (times/until),
2) whether it should emit elements it passes by, and 3) which search algorithm to use (depth-first or breadth-first).

By default it will continue repeating until there's no more results, not emit anything along the way, and use
depth first search, i.e. go deep before wide.

Here are some popular alternative behaviour configurations:

.repeat(_.out)(_.times(3))                               // perform exactly three repeat iterations
.repeat(_.out)(_.until(_.property(Name).endsWith("2")))  // repeat until the 'Name' property ends with '2'
.repeat(_.out)(_.emit)                                   // emit everything along the way
.repeat(_.out)(_.emit.breadthFirstSearch)                // emit everything, use BFS
.repeat(_.out)(_.emit(_.property(Name).startsWith("L"))) // emit if the 'Name' property starts with 'L'

See RepeatTraversalTests for more examples.

Note that this works for domain-specific steps as well as generic graph steps - for details please take a look at
the examples in RepeatTraversalTests: both {{{.followedBy}}} and {{{.out}}} work.

@mpollmeier mpollmeier requested review from fabsx00 and ml86 July 2, 2020 02:55
object DepthFirst {
case class StackItem[A](traversal: Traversal[A], depth: Int)

def apply[B](repeatTraversal: B => Traversal[B], behaviour: RepeatBehaviour[B]): B => Traversal[B] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am kind of surprised to see two completely different implementations for DepthFirst and BreadthFirst traversal. I would have expected the difference to only be the collection type for the "worklist" which would be a stack for depth first and a FIFO for breadth first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes good point, this is mostly an artifact of my journey to the final version. Initially the two versions looked very similar, but the DFS version used recursion, i.e. the worklist stack was the JVM call stack. Until I realised that that's not a good idea because it quickly blows up.
Will TAL tomorrow to see if it's worthwhile to bring the two impls closer together. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup you're right. i'll still merge this as is to have this version archived in the git history, will send a follow up pr then to unify the two options

@mpollmeier mpollmeier merged commit 212a5a9 into master Jul 2, 2020
@delete-merged-branch delete-merged-branch bot deleted the michael/repeat-step branch July 2, 2020 22:18
mpollmeier added a commit that referenced this pull request Jul 2, 2020
mpollmeier added a commit that referenced this pull request Jul 5, 2020
…119)

* refactor

* refactor repeat implementation: use same implementation for BFS/DFS

re #118 (comment)

also improves scaladoc
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