You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2022/01/19 12:35:39 UTC

[GitHub] [tinkerpop] FlorianHockmann commented on a change in pull request #1551: Fixing .NET test failures and adding Gherkin/.NET support for VertexProperty.

FlorianHockmann commented on a change in pull request #1551:
URL: https://github.com/apache/tinkerpop/pull/1551#discussion_r787486627



##########
File path: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
##########
@@ -133,6 +137,70 @@ public ScenarioData(IMessageSerializer messageSerializer)
             }
         }
 
+        private static IDictionary<string, VertexProperty> GetVertexProperties(GraphTraversalSource g)
+        {
+            try
+            {
+                /*
+                 * This closure will turn a VertexProperty into a triple string of the form:
+                 * "vertexName-propKey->propVal"
+                 *
+                 * It will also take care of wrapping propVal in the appropriate Numeric format. We must do this in
+                 * case a Vertex has multiple properties with the same key and number value but in different numeric
+                 * type spaces (rare, but possible, and presumably something we may want to write tests around).
+                 */
+                string groovy = @"

Review comment:
       (nitpick) Really not important, but you can most of the time just use `var` instead of the concrete type of a variable as that can be interfered from its initialization. So, `var groovy = [...]` here.
   This makes it a bit nicer to read in my opinion.

##########
File path: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
##########
@@ -133,6 +137,70 @@ public ScenarioData(IMessageSerializer messageSerializer)
             }
         }
 
+        private static IDictionary<string, VertexProperty> GetVertexProperties(GraphTraversalSource g)
+        {
+            try
+            {
+                /*
+                 * This closure will turn a VertexProperty into a triple string of the form:
+                 * "vertexName-propKey->propVal"
+                 *
+                 * It will also take care of wrapping propVal in the appropriate Numeric format. We must do this in
+                 * case a Vertex has multiple properties with the same key and number value but in different numeric
+                 * type spaces (rare, but possible, and presumably something we may want to write tests around).
+                 */
+                string groovy = @"
+                    { vp ->
+                          def result = vp.element().value('name') + '-' + vp.key() + '->'
+                          def value = vp.value()
+                          def type = ''
+                          switch(value) {

Review comment:
       Wow, a lot of effort to get these scenarios to work in .NET 🙈 

##########
File path: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Remote/RemoteStrategyTests.cs
##########
@@ -1,4 +1,4 @@
-#region License

Review comment:
       Hmm, the good old BOM byte. Maybe we should add an `.editorconfig` to prevent these from being added back again?
   (Not suggesting that something like that should be added as part of this PR, just something that came to my mind while seeing this here.)

##########
File path: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/ScenarioData.cs
##########
@@ -133,6 +137,70 @@ public ScenarioData(IMessageSerializer messageSerializer)
             }
         }
 
+        private static IDictionary<string, VertexProperty> GetVertexProperties(GraphTraversalSource g)
+        {
+            try
+            {
+                /*
+                 * This closure will turn a VertexProperty into a triple string of the form:
+                 * "vertexName-propKey->propVal"
+                 *
+                 * It will also take care of wrapping propVal in the appropriate Numeric format. We must do this in
+                 * case a Vertex has multiple properties with the same key and number value but in different numeric
+                 * type spaces (rare, but possible, and presumably something we may want to write tests around).
+                 */
+                string groovy = @"
+                    { vp ->
+                          def result = vp.element().value('name') + '-' + vp.key() + '->'
+                          def value = vp.value()
+                          def type = ''
+                          switch(value) {
+                            case { !(it instanceof Number) }:
+                              return result + value
+                            case Byte:
+                              type = 'b'
+                              break
+                            case Short:
+                              type = 's'
+                              break
+                            case Integer:
+                              type = 'i'
+                              break
+                            case Long:
+                              type = 'l'
+                              break
+                            case Float:
+                              type = 'f'
+                              break
+                            case Double:
+                              type = 'd'
+                              break
+                            case BigDecimal:
+                              type = 'm'
+                              break
+                            case BigInteger:
+                              type = 'n'
+                              break
+                          }
+                          return result + 'd[' + value + '].' + type
+                    }  
+                ";
+                
+                IFunction lambda = Lambda.Groovy(groovy, 1);
+
+                IDictionary<string, VertexProperty> dict = g.V().Properties<VertexProperty>().Group<string, VertexProperty>()

Review comment:
       Same nit here basically:
   ```suggestion
                   var dict = g.V().Properties<VertexProperty>().Group<string, VertexProperty>()
   ```




-- 
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-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org