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

Added the negative cycle detection in bellman ford #97

Merged
merged 6 commits into from
Sep 25, 2023

Conversation

unbalancedvariance
Copy link
Contributor

@unbalancedvariance unbalancedvariance commented Sep 21, 2023

This PR solves the #76 issue.

I have added the code for detecting a Negative cycle in Bellman Ford Shortest Path Algorithm and I have also added a unit test to test the logic in the test directory.

I did an additional pass over the edges and if there is any change detected ,An Invalid Argument exception is thrown.
"A Negative cycle has been detected in the graph".The unit test checks if this exception is thrown for a negative cycle and all the tests are passed too.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there! Thank you for creating your first pull-request on the Graaf library :)

@bobluppes bobluppes self-requested a review September 24, 2023 09:17
@bobluppes bobluppes added the enhancement New feature label Sep 24, 2023
@codecov
Copy link

codecov bot commented Sep 24, 2023

Codecov Report

Patch coverage is 100.00% of modified lines.

Files Changed Coverage
include/graaflib/algorithm/shortest_path.tpp 100.00%
test/graaflib/algorithm/shortest_path_test.cpp 100.00%

📢 Thoughts on this report? Let us know!.

Copy link
Owner

@bobluppes bobluppes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution, looks very good already!
Should be almost ready to merge, just some minor comments.
Let me know what you think.

@unbalancedvariance
Copy link
Contributor Author

Hey, Thanks for the detailed review, I have committed all the above changes. Let me know if any more changes are needed.

Copy link
Owner

@bobluppes bobluppes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Congrats on the first contribution :)

@bobluppes bobluppes merged commit 5ae6f61 into bobluppes:main Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants