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

Remove needless Edge\UndirectedId #46

Merged
merged 4 commits into from
Jul 3, 2013
Merged

Remove needless Edge\UndirectedId #46

merged 4 commits into from
Jul 3, 2013

Conversation

clue
Copy link
Member

@clue clue commented Jun 13, 2013

Remove experimental leftover

  • Always use Edge\Undirected instead of Edge\UndirectedId and remove Edge\UndirectedId
  • 100% code coverage
  • Documentation / changelog

Refs #27.

@clue
Copy link
Member Author

clue commented Jun 13, 2013

Afaikt, this does not affect BC and does not need an addition to the changelog? The documentation for Vertex::createEdgeTo() has always referred to EdgeUndirected anyway.

Remove [WIP] marker, ready for review. I'll leave this open for a day or two, but this should be safe to be merged as-is.

@clue
Copy link
Member Author

clue commented Jun 13, 2013

Just to add some context: The Edge\UndirectedId was the result of an experiment creating a graph with 100.000 vertices and 200.000 edges. PHP (version unclear, 5.3.?) used to segfault when we tested this around a year ago. It's been worked around by reducing the number of circular references, but imo this was never more than an experiment anyway. As such, we don't have any bug reports, detailed information or unit tests to reproduce this issue.

And in my opinion, if you're trying to work with Graphs that big, you're probably better off with using a full-fledged graph database instead of keeping the whole graph in memory (independent of the underlying graph library).

@clue
Copy link
Member Author

clue commented Jul 3, 2013

Rebased on current master, fixed a now relevant reference in Algorithm\Tree\Undirected and added to README just in case.

clue added a commit that referenced this pull request Jul 3, 2013
Remove needless Edge\UndirectedId
@clue clue merged commit 498598b into master Jul 3, 2013
@clue clue deleted the remove-edge-undirectedid branch July 3, 2013 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant