search

arangodb / arangodb-php

PHP ODM for ArangoDB
https://www.arangodb.com
Apache License 2.0
183 stars 46 forks source link

saveEdge() should allow $document to be null #68

Open F21 opened 11 years ago

F21 commented 11 years ago

Many times, I just need to create a simple edge between 2 vertices with a label.

In this case, I do not need to set attributes of any type on an edge and it is cumbersome to manually instantiate that edge.

Currently, the method signature for saving an edge is:

public function saveEdge($graphName, $from, $to, $label = null, $document)

I propose that we set it to this:

public function saveEdge($graphName, $from, $to, $label = null, $document =null)

If $document is null, the method can simply create a new edge automatically. The only problem is that we might want the edge object back, so in this case, it will need to return the edge object.

Comments welcome :smile:

frankmayer commented 11 years ago

Hi, yes, that's a good idea. Why not. Waiting for your PR :smile:

F21 commented 11 years ago

@frankmayer: There are some problems though. Currently saveEdge() returns the ID of the edge. If we implement this, we need to return the edge (because people might want to use this edge).

I don't feel very comfortable with having an ID returned when a document is provided and a document returned when it is not.

Any thoughts?

frankmayer commented 11 years ago

Yes, That's a totally different approach to what the driver had been taking from the beginning. The only way I see, to include this, is that the user chooses to get either the id or the object, by setting an option.

F21 commented 11 years ago

Ah. However, that means there are lots of "configuration" to set if we take that approach. Then things become brittle, because if they decide to change the configuration to return the id in the future, any code that uses saveEdge() will need to change :dizzy_face: and things will be hard to debug.

Perhaps we will should leave this for now and wait for #50, because I feel that every PR/new feature I contribute means that I need to supports backwards compatibility which means that I introduce more configuration options and so on, and the API becomes less beautiful than before :stuck_out_tongue: