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

Explicitly convert graph vertices to string #56

Merged
merged 1 commit into from
Jun 7, 2014

Conversation

eush77
Copy link
Contributor

@eush77 eush77 commented Jun 7, 2014

The (minor) problem with current graph implementation is that vertices property contains vertices as they were introduced to addVertex, but neighbors automatically convert them to string (because internally vertices are used as object keys).

It can lead to some strange results (note the types).

var graph = new Graph();
graph.addEdge(5, 0);
graph.addEdge(0, 2);
graph.addEdge(2, 4);
graph.addEdge(4, 0);
graph.addEdge(1, 5);
graph.addEdge(3, 1);
graph.addEdge(5, 3);

var trail = eulerPath(graph);
// [ 5, '3', '1', 5, '0', '2', '4', 0 ]

I understand that this “class” was designed for string vertices in mind, but it is rather common for vertices to be integers as well. String type makes it also easier to implement. The smallest possible fix I can think of is to convert vertices in graph.vertices to strings.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling d98b7d5 on eush77:graph-vertex-type-fix into 5a6129b on felipernb:master.

felipernb added a commit that referenced this pull request Jun 7, 2014
Explicitly convert graph vertices to string
@felipernb felipernb merged commit 6689b98 into felipernb:master Jun 7, 2014
@eush77 eush77 deleted the graph-vertex-type-fix branch June 7, 2014 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants