Topological Sorting #93
Conversation
Corrected trailling slashes
- relative railway example - corrected png files
- Added test cases and docs
Codecov ReportPatch coverage is
📢 Thoughts on this report? Let us know!. |
|
Hey @joweich, thanks for your comments. I'll fix it. :) |
| processed_vertices[start_vertex] = vertex_status::VISITED; | ||
| for (const auto& next_vertex : graph.get_neighbors(start_vertex)) { | ||
| if (processed_vertices[next_vertex] == vertex_status::UNVISITED) { | ||
| do_dfs_topological_sort(graph, next_vertex, processed_vertices, | ||
| sorted_vertices); | ||
| } | ||
| } | ||
|
|
||
| sorted_vertices.push_back(start_vertex); |
There was a problem hiding this comment.
If you add the start_vertex to the sorted_vertices before the for-loop, do you already have the vertices recorded in the correct order? Maybe it would be possible to get rid of the std::reverse then.
There was a problem hiding this comment.
I'm not sure about that. In this case, vertices will be stored as usual in DFS traversal, ignoring all other dependencies.
Consider this directed graph
0
/ \
1 2
\ /
3
If we do preorder, the vertices will be 0–1–3–2, which is incorrect for TS.
But if we do post order 0-1-2-3, Or maybe I missed something, correct me if I'm getting something wrong.
There was a problem hiding this comment.
I quickly checked @bobluppes's proposal and it indeed fails the tests. I would suggest going with the solution in this PR and think about ways getting rid of the std::reverse in a follow up.
There was a problem hiding this comment.
Ah, my bad indeed. One thing we could investigate is to use a std::deque rather than a vector and call push_front. However, let's not optimize prematurely and let's tackle this in a follow up.
Added one test
| ``` | ||
|
|
||
| - **graph** The directed graph to traverse. | ||
| - **return** Vector of vertices sorted in topological order. If the graph contains cycles, it returns an empty vector. |
There was a problem hiding this comment.
| - **return** Vector of vertices sorted in topological order. If the graph contains cycles, it returns an empty vector. | |
| - **return** Vector of vertices sorted in topological order. If the graph contains cycles, it returns std::nullopt. |
| processed_vertices[start_vertex] = vertex_status::VISITED; | ||
| for (const auto& next_vertex : graph.get_neighbors(start_vertex)) { | ||
| if (processed_vertices[next_vertex] == vertex_status::UNVISITED) { | ||
| do_dfs_topological_sort(graph, next_vertex, processed_vertices, | ||
| sorted_vertices); | ||
| } | ||
| } | ||
|
|
||
| sorted_vertices.push_back(start_vertex); |
There was a problem hiding this comment.
I quickly checked @bobluppes's proposal and it indeed fails the tests. I would suggest going with the solution in this PR and think about ways getting rid of the std::reverse in a follow up.
Corrected doc file
Added TS DFS-based.