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

Support native cloning of Base objects #58

Closed
clue opened this issue Jul 10, 2013 · 2 comments
Closed

Support native cloning of Base objects #58

clue opened this issue Jul 10, 2013 · 2 comments

Comments

@clue
Copy link
Member

clue commented Jul 10, 2013

Support native cloning of Graph, Vertex and Edge.

@clue clue changed the title Enable *safe* cloning of Base objects Support native cloning of Base objects Mar 8, 2015
@clue
Copy link
Member Author

clue commented Mar 8, 2015

The vertex ID is currently unique within the whole graph. This means that we can not create an exact clone of the vertex within the same graph. This is what currently blocks this ticket.

In the linked PR #89 @clemens-tolboom suggested assigning a new vertex ID when cloning a Vertex instance. This would allow us to work around this limitation. I'm not perfectly happy with this tbh, but perhaps this is really only a cosmetic issue if done right.

As an alternative, ticket #131 questions whether we actually need the vertex ID in the first place and suggests dropping it altogether. Voilà, cloning made easy.

Any input would be welcome!

@clue clue added this to the v0.11.0 milestone Oct 3, 2018
@clue clue modified the milestones: v0.11.0, v1.0.0 Sep 29, 2019
@clue
Copy link
Member Author

clue commented Oct 4, 2019

Solving this isn't too hard from a technical perspective, but I've decided against putting any more time into this simply because its API is somewhat unclear. Take the following example:

$graph = new Graph();
$v1 = $graph->createVertex();
$v2 = clone $v1;

assert(count($graph->getVertices()) === 2); // ???

I don't think the suggest behavior makes a lot of sense, so I really don't think native cloning provides a sufficiently reasonable API. the existing API methods such as createVertexClone() make this behavior explicit 👍

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

Successfully merging a pull request may close this issue.

1 participant