-
Notifications
You must be signed in to change notification settings - Fork 35
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
Work on Tarjan's SCC algorithm #78
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #78 +/- ##
==========================================
- Coverage 99.74% 99.57% -0.17%
==========================================
Files 23 25 +2
Lines 1158 1408 +250
==========================================
+ Hits 1155 1402 +247
- Misses 3 6 +3
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, LGTM!
I will commit the minor suggestions, after which I will merge the PR. So no need to address them.
docs/docs/algorithms/strongly-connected-components/_category_.json
Outdated
Show resolved
Hide resolved
// Lambda function for the strong connect traversal | ||
std::function<void(vertex_id_t)> strong_connect; | ||
|
||
strong_connect = [&](vertex_id_t vertex) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[very minor]: Since we are not passing around the lambda in an interface, we can keep it a lambda closure rather than assigning it to a function object:
// Lambda function for the strong connect traversal | |
std::function<void(vertex_id_t)> strong_connect; | |
strong_connect = [&](vertex_id_t vertex) { | |
// Lambda function for the strong connect traversal | |
const auto strong_connect = [&](vertex_id_t vertex) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see now why you did this, since the function object is used in the body the compiler needs to know the type. Reverted this change in 35036c3.
It might also be interesting to see if we can implement this in terms of the existing traversal algorithms in graph_traversal.h
, but I will leave this for a follow up.
See #53