[ 
https://issues.apache.org/jira/browse/TINKERPOP-1752?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16182717#comment-16182717
 ] 

ASF GitHub Bot commented on TINKERPOP-1752:
-------------------------------------------

Github user FlorianHockmann commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/712#discussion_r141376043
  
    --- Diff: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Bytecode.cs ---
    @@ -79,7 +85,77 @@ public void AddSource(string sourceName, params object[] 
args)
             /// <param name="args">The traversal method arguments.</param>
             public void AddStep(string stepName, params object[] args)
             {
    -            StepInstructions.Add(new Instruction(stepName, args));
    +            StepInstructions.Add(new Instruction(stepName, 
FlattenArguments(args)));
    +            Bindings.Clear();
    --- End diff --
    
    That's a good point. So we should probably support lambdas. To be honest, I 
haven't really looked into what would be necessary to support lambdas. If I'm 
not mistaken then we can easily support Python and Groovy lambdas when we add a 
dedicated GraphSON serializer for them and probably a class that wraps the 
lambda together with its language. Then the steps that take a lambda would 
simply accept an object of that class in Gremlin.Net. I can create a separate 
ticket for this when we agree that we want to support them.
    
    When we want to support lambdas then we should also support `Bindings`. 
That leaves us with the question of whether we find a better implementation 
that doesn't suffer from the concurrency problems @jorgebay mentioned or if the 
current implementation in this pull request is acceptable.
    
    Apart from that, shouldn't we clarify the section in the documentation 
about bindings? It currently states for example [for 
gremlin-python](http://tinkerpop.apache.org/docs/current/reference/#_bindings):
    
    > If a traversal is going to be executed repeatedly, but with different 
parameters, then bindings should be used.
    
    because:
    > translation and compilation can be skipped as the previously compiled 
version should be cached
    
    which is only true when a lambda is used if I understood it correctly.


> Gremlin.Net: Generate completely type-safe methods
> --------------------------------------------------
>
>                 Key: TINKERPOP-1752
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-1752
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: dotnet
>    Affects Versions: 3.2.5
>            Reporter: Florian Hockmann
>            Priority: Minor
>
> Currently the generated traversal methods in Gremlin.Net take {{params 
> object[] args}} as an argument which allows the user to provide an arbitrary 
> number of arguments with any type. While this makes the generation rather 
> simple, it doesn't tell the user which arguments are actually valid so users 
> can submit completely invalid traversals like:
> {code}
> g.V(1).AddE(1234, "invalidArgument2").Next()
> {code}
> Type-safe methods could also use the original argument names to tell users 
> something about what kind of values the methods expect. Consider for example 
> the following method signatures for the C# step {{AddE}} that are basically a 
> 1:1 representation of the original Java {{addE}} step:
> {code}
> public GraphTraversal< S , Edge > AddE (Direction direction, string 
> firstVertexKeyOrEdgeLabel, string edgeLabelOrSecondVertexKey, params object[] 
> propertyKeyValues);
> public GraphTraversal< S , Edge > AddE (string edgeLabel);
> {code}
> Implementing this should make TINKERPOP-1725 obsolete and also resolve 
> TINKERPOP-1751.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to