FlorianHockmann commented on a change in pull request #1557:
URL: https://github.com/apache/tinkerpop/pull/1557#discussion_r804526525
##########
File path: gremlin-dotnet/src/Gremlin.Net/Process/Traversal/P.cs
##########
@@ -85,79 +85,79 @@ public P Or(P otherPredicate)
public static P Between(params object[] args)
{
- var value = args.Length == 1 ? args[0] : args;
+ var value = args != null && args.Length == 1 ? args[0] : args;
return new P("between", value);
}
public static P Eq(params object[] args)
{
- var value = args.Length == 1 ? args[0] : args;
+ var value = args != null && args.Length == 1 ? args[0] : args;
return new P("eq", value);
}
public static P Gt(params object[] args)
{
- var value = args.Length == 1 ? args[0] : args;
+ var value = args != null && args.Length == 1 ? args[0] : args;
Review comment:
Hey Mike, sorry if this came off the wrong way! I didn't have much time
on Wednesday to review this and therefore really only checked the .NET changes.
So I didn't see all the work you put into comparability semantics, including
handling of `null`.
I just read through (most) of the added docs where you describe the improved
semantics and checked the added feature tests. Now I understand why it makes
sense to allow `null` here and I definitely welcome these changes! It's good to
have a clear definition of how we handle infinity and `null`.
This will also help with TINKERPOP-2348 where we want to clearly communicate
to users of Gremlin.NET where `null` is allowed as an argument or a possible
return value and where not. When I initially worked on that, I ran into the
problem that I wasn't sure in a lot of places whether we actually want to
accept `null` there or not. So, any clarifications we make on how we handle
`null` in various places are definitely welcome 😄
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
