[ https://issues.apache.org/jira/browse/TINKERPOP-2302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16941236#comment-16941236 ]
ASF GitHub Bot commented on TINKERPOP-2302: ------------------------------------------- dalaro commented on pull request #1203: TINKERPOP-2302 add `ElementMapStep#isOnGraphComputer()` (tp34) URL: https://github.com/apache/tinkerpop/pull/1203 Adds a getter for `onGraphComputer`. This is intended to support strategies that replace this step but want to preserve this state across replacement. https://issues.apache.org/jira/browse/TINKERPOP-2302 ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Add isOnGraphComputer() field accessor to ElementMapStep > -------------------------------------------------------- > > Key: TINKERPOP-2302 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2302 > Project: TinkerPop > Issue Type: Improvement > Components: process > Affects Versions: 3.5.0, 3.4.4 > Reporter: Dan LaRocque > Priority: Minor > > TINKERPOP-2284 added {{ElementMapStep}}. > This step implements {{GraphComputing}}, so it defines that interface's > {{onGraphComputer()}} method. The method sets a private boolean field. > For any implementation that wants to alter {{ElementMapStep}} behavior by > replacing instances of the step with an alternative implementation (maybe a > subclass), it could be convenient to make the private {{onGraphComputer}} > state accessible. I think the rest of {{ElementMapStep}}'s public state is > already accessible; it's just this field that is private. > There are a few ways to go about this. > * We could add an {{isOnGraphComputer()}} method to just {{ElementMapStep}}, > or make {{onGraphComputer}} protected. This is the narrowest version of the > change. > * We could add an {{isOnGraphComputer()}} method to the {{GraphComputing}} > interface. This would affect a bunch of implementing steps. Most > implementations on tp34 have an internal boolean field storing this state, so > it would be easy on those. However, [{{GraphStep}} does not have a boolean > onGraphComputer > field|https://github.com/apache/tinkerpop/blob/tp34/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java#L131-L135]: > {code} > @Override > public void onGraphComputer() { > this.iteratorSupplier = Collections::emptyIterator; > convertElementsToIds(); > } > {code} > We could maybe implement this without adding another field, but it would > probably be simpler to add a field than to infer it from existing state. > *Altering this interface would also break any third-party implementations of > {{GraphComputing}}.* > * We could create a {{GraphComputing}} subinterface that adds an > {{isOnGraphComputer()}}, and only implement it where a preexisting boolean > field makes the job easy. This somewhere in between the first two approaches > in terms of scope. > I'm going to open a PR taking the first, narrowest possible approach, where > I'm only touching {{ElementMapStep}}. I'd be happy to rework it to one of > the alternatives, if desired. -- This message was sent by Atlassian Jira (v8.3.4#803005)