andreachild commented on code in PR #3209:
URL: https://github.com/apache/tinkerpop/pull/3209#discussion_r2370888205
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParent.java:
##########
@@ -31,14 +33,31 @@
import java.util.Set;
/**
+ * Indicates that a step can contain child Traversals. Any step which
implements this interface should override at least
Review Comment:
Nit: Likely out of scope for this PR but I feel it would be helpful in
general for TinkerPop to have more custom annotations. In this case there could
be a custom annotation like `@ChildTraversal(scope=Type.LOCAL)` to mark step
fields that store a child traversal. Then there can be a single common method
which handles the presence of child traversals in `AbstractStep.java` like
(pseudocode):
```
@Override
public void setTraversal(final Traversal.Admin<?, ?> traversal) {
this.traversal = traversal;
// retrieve any step fields annotated with @ChildTraversal
// for each child traversal, call integrateChild
}
```
And then there could also be an `AbstractTravesalParent.java` which has:
```
<S, E> List<Traversal.Admin<S, E>> getGlobalChildren() {
// use reflection to retrieve fields annotated with
@ChildTraversal(type=GLOBAL)
}
<S, E> List<Traversal.Admin<S, E>> getLocalChildren() {
// use reflection to retrieve fields annotated with
@ChildTraversal(type=LOCAL)
}
```
Then when adding new steps or adding child traversals to an existing step,
all you would need to do is add the custom annotation and you wouldn't have to
'remember' to do the boilerplate logic.
If you agree with this suggestion it's possible to use an annotation for
these affected classes only and then slowly migrate towards 100% usage of the
annotation in future PRs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
