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

TINKERPOP-1471: IncidentToAdjacentStrategy use hidden marker to avoid repeated recursion. #474

Merged
merged 2 commits into from
Nov 3, 2016

Conversation

okram
Copy link
Contributor

@okram okram commented Nov 1, 2016

https://issues.apache.org/jira/browse/TINKERPOP-1471

Hidden labels can be used by strategies as a way to communicate between "layers" of the compilation tree. This model is now applied to IncidentToAdjacentStrategy where if the traversal (as a whole) should NOT be processed by IncidentToAdjacentStrategy, then each traversal in the tree is "marked." If the traversal can be processed by IncidentToAdjacentStrategy, then there is no need to mark the traversals. This removes the need to, for every traversal, do a recursion from the parent traversal looking for invalidating step classes.

VOTE +1.

…he need for constant rootTraversal recrussion in search of invalidating steps.
@dkuppitz
Copy link
Contributor

dkuppitz commented Nov 1, 2016

VOTE: +1

A little refactoring would be nice though:

for (final Step<?, ?> step : traversal.getSteps()) {
    if (step instanceof TraversalParent) {
        ((TraversalParent) step).getLocalChildren().forEach(this::addLabelMarker);
        ((TraversalParent) step).getGlobalChildren().forEach(this::addLabelMarker);
    }
}

... should be as easy as:

TraversalHelper.forEachNestedTraversal(traversal, this::addLabelMarker);

…dkuppitz. Was apply to replace code in both SubgraphStrategy and IncidentToAdjacentStrategy.
@@ -105,21 +105,11 @@ private SubgraphStrategy(final Traversal<Vertex, ?> vertexCriterion, final Trave
this.vertexPropertyCriterion = null == vertexPropertyCriterion ? null : vertexPropertyCriterion.asAdmin().clone();

if (null != this.vertexCriterion)
this.addLabelMarker(this.vertexCriterion);
TraversalHelper.applyTraversalRecursively(t -> t.getStartStep().addLabel(MARKER), this.vertexCriterion);
Copy link
Contributor

Choose a reason for hiding this comment

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

vertexCriterion can be null. In that case TraversalHelper.applyTraversalRecursively and/or the consumer would throw a NPE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

? -- look at line 107 -- if(null != this.vertexCriteria)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ha, it's getting late here.....

@spmallette
Copy link
Contributor

All tests pass with docker/build.sh -t -n -i

VOTE +1

@asfgit asfgit merged commit 1dcca1b into tp32 Nov 3, 2016
@asfgit asfgit deleted the TINKERPOP-1471 branch February 21, 2017 14:50
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