addVertex(Object... keyValues) design issue

[Joan]

Hi,

I'm a contributor of the Gremlin-Scala lib and we always had issues with the following definition in the Graph interface:

public Vertex addVertex(final Object... keyValues);

I always thought it was a design mistake as it creates ambiguous reference to overloaded definition even without talking about Gremlin-Scala but just with addVertex(String).

And I'm not even talking about the 0 type checking at compile time with this.

So are you planning to change that to something like that:

public class KeyValue {
    public final String key;
    public final Object value;

    public KeyValue(String key, Object value) {
        this.key = key;
        this.value = value;
    }
}

public Vertex addVertex(final KeyValue... keyValues);

???

This more verbose but I think the verbosity is a common issue in the Java world.

Cheers

[Daniel Kuppitz]

Hi Joan,

first: the key is not necessarily a String (e.g. T.id, T.label). Next: Why is the method ambiguity a problem? Java's selection algorithm is pretty clear:

"There may be more than one such method, in which case the most specific one is chosen."

I wouldn't expect any difference in Scala. Does the Scala compiler complain about it?

Cheers,

Daniel

 

--
You received this message because you are subscribed to the Google Groups "Gremlin-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to gremlin-user...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/gremlin-users/1d55285f-f0d4-44ba-82ae-6c511218df35%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Joan]

Actually this is an interoperability bug between Scala en Java.

Well that sounded dodgy to me to have a method that takes all types and as many as we want and fail at runtime if it's not the good type or if it's not even.

For the key that can be a T, we could have 2 KeyValue class that inherit from an interface, one for each type (String or T).

But I was mainly complaining about the first issue that is apparently a Scala issue.

Cheers

[Matan Safriel]

Joan, what is the exact interoperability bug between Scala and Java here?

I do indeed see special handling in gremlin-scala, e.g. in:

  def addVertex(properties: (String, Any)*): ScalaVertex = {
    val params = properties.flatMap(pair ⇒ Seq(pair._1, pair._2.asInstanceOf[AnyRef]))
    graph.addVertex(params)
  }

And I wonder why the coercion to AnyRef is required for Scala's Seq of the variable number of arguments, and not for a Scala Map as in:

def addVertex(properties: Map[String, Any]): ScalaVertex =
    addVertex(properties.toSeq: _*)

Both, in https://github.com/mpollmeier/gremlin-scala/blob/master/gremlin-scala/src/main/scala/gremlin/scala/ScalaGraph.scala

Maybe Scala should get an object constructor like Java 8....

Matan

[Joan Goyeau]

Hi,

The code you quoted is not linked to the issue I mentioned. In the quoted code we are just forcing the boxing of AnyVal (all primitive values) to an AnyRef (alias to the java Object). And that code is fine.
The map version is calling the scala addVertex (overloading). So the second quote is calling the first one.

The issue I am talking about is this one:
https://issues.scala-lang.org/plugins/servlet/mobile#issue/SI-4775
And this is due to Scala only.

Cheers

You received this message because you are subscribed to a topic in the Google Groups "Gremlin-users" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/gremlin-users/M7knBWSHVi4/unsubscribe.
To unsubscribe from this group and all its topics, send an email to gremlin-user...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/gremlin-users/6c065970-3808-4508-91df-2e945c9b68f4%40googlegroups.com.

[Matan Safriel]

Oh. Ouch.

To view this discussion on the web visit https://groups.google.com/d/msgid/gremlin-users/CAJYfrs1GmFAer34OfAxXqMbMYn%3DP_ivJZwqa8TU86mWyUNAx0Q%40mail.gmail.com.

[Matt Frantz]

What if we create the following overloads:

Vertex addVertex();

Vertex addVertex(String label);

Vertex addVertex(Object key, Object value, Object...keyValues);

This would avoid the 1-arg overload, since there is only one 1-arg variant.  It also makes the key/value structure more obvious.

BTW, the JavaDoc now says "...the odd numbered arguments are {@link String} property keys," so if we actually allow other types, that doc should be fixed.

Would that help? 

[Matan Safriel]

I inquired in SI-4775 whether this is solvable from the Scala language side anytime soon. Otherwise this would probably make it much easier for folks to use gremlin 3 from Scala without being bound to the specific scala-gremlin implementation. A questions though: Will a non-string really work? it feels like evading type safety the way objects need to be passed in, in tuples. I wonder why this particular api design. Can you point me at the current implementations of the addVertex interface? I got lost navigating the code...

--

You received this message because you are subscribed to a topic in the Google Groups "Gremlin-users" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/gremlin-users/M7knBWSHVi4/unsubscribe.
To unsubscribe from this group and all its topics, send an email to gremlin-user...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/gremlin-users/eb2a451a-af66-48a1-989c-8021473647fc%40googlegroups.com.

[Joan]

Hi Matan Safriel,

A questions though: Will a non-string really work?

Are you asking this question for Gremlin-Scala?

The reason why Gremlin-Scala will still most probably be preferred by Scala users is because there is lot's of little but very welcomed language feature adaptation like tuples, type safety in traversals, compile time mapping of objects to vertex...

But it's true that with Java 8 we are getting closer to Scala and that's good.

Cheers

[Matt Frantz]

https://issues.apache.org/jira/browse/TINKERPOP3-850

[Matt Frantz]

Joan,

Will you please take a look at this PR to see if the API change is acceptable to avoid ambiguous overloads in Scala?

https://github.com/apache/incubator-tinkerpop/pull/101

Can Scala distinguish between the String and the Object[] overloads?