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

Possible bug in the method for Eulerian paths #312

Closed
sbaldu opened this issue May 26, 2023 · 2 comments · Fixed by #314
Closed

Possible bug in the method for Eulerian paths #312

sbaldu opened this issue May 26, 2023 · 2 comments · Fixed by #314
Assignees
Labels
bug Something isn't working core something about core Priority:Medium Priority Label for medium priority issue

Comments

@sbaldu
Copy link
Collaborator

sbaldu commented May 26, 2023

In the Graph::eulerianPath method the starting node is chosen as that pointed by nodeSet.begin(). This works in the test cases, but if we change the order of the nodes then it doesn't work anymore.
The starting node for a directed graph in Hierholzer’s Algorithm is the node which has more outgoing links than the others, so I would add this condition for choosing the starting node. For undirected graphs it doesn't make any difference since the choice of the starting node is arbitrary.

Please let me know if you agree or if I am mistaken.

@nrkramer
Copy link
Collaborator

Please let me know if you agree or if I am mistaken.

Your analysis seems correct to me. Wdyt @ZigRazor ?

@sbaldu sbaldu self-assigned this May 27, 2023
@ZigRazor
Copy link
Owner

I also think it is correct!

@ZigRazor ZigRazor added bug Something isn't working core something about core Priority:Medium Priority Label for medium priority issue labels May 29, 2023
@ZigRazor ZigRazor linked a pull request May 29, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core something about core Priority:Medium Priority Label for medium priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants