Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
F21 opened this issue Mar 31, 2013 · 4 comments
Open

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

F21 opened this issue Mar 31, 2013 · 4 comments

Comments

@F21
Copy link
Contributor

F21 commented Mar 31, 2013

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 😄

@frankmayer
Copy link
Contributor

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

@F21
Copy link
Contributor Author

F21 commented Apr 1, 2013

@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
Copy link
Contributor

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
Copy link
Contributor Author

F21 commented Apr 2, 2013

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 😵 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 😛

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants