Improved return type for Kosaraju's algoritm + tests

[dvd2000]

I implemented the changes discussed in #251 concerning the return type of Kosaraju's algorithm (I hope this correspond to what you had in mind) and written a few tests for it (#221).

For the return type, I introduced a SCCResult type (Strongly Connected Components Result), similar to what has been done for other algorithms in the library. Let me know if I can improve my PR in some way.

[ghost]

👇 Click on the image for a new way to code review

• Make big changes easier — review code in small groups of related files

• Know where to start — see the whole change at a glance

• Take a code tour — explore the change with an interactive tour

• Make comments and review — all fully sync’ed with github

  Try it now!

Legend

[dvd2000]

I think I made some mistakes when merging to keep my fork updated, sorry
I'll fix that tomorrow

EDIT: Should be fixed now

[ZigRazor]

Good Work @dvd2000 !

[dvd2000]

Glad I could contribute.

I'm still not completely convinced about the return type though. Specifically:

• Wouldn't it be better if the returned connected components contained pointers to the vertex in the graph, instead of (subgraph) copies? I mean, if one wants to check which component a given vertex of the graph belongs to, I think it would make more sense to refer to vertices in the original graph than to vertices in a graph isomorphic/identical to the original. But this question applies to other algorithms in the library too (e.g. BFS, DFS). In some cases (e.g. Max flow, MST algorithms) we return vectors of pairs of strings for the edges, why don't we return vectors of pointers to edges instead? I general, I feel that the interface is a bit too heterogeneous concerning algorithms returns type.

• With this choice of data structure (i.e. std::vector) it is kind of hard to get the strongly connected component for a given vertex (linear time search). Maybe doing something like what has been done for DialResult (i.e. unordered_map) could simplify this. And providing hashing for single vertices is not really a problem since they already have an id attribute.

[ZigRazor]

I think we can close this PR, and then rework it in a secondary issue.

• Wouldn't it be better if the returned connected components contained pointers to the vertex in the graph, instead of (subgraph) copies? I mean, if one wants to check which component a given vertex of the graph belongs to, I think it would make more sense to refer to vertices in the original graph than to vertices in a graph isomorphic/identical to the original. But this question applies to other algorithms in the library too (e.g. BFS, DFS). In some cases (e.g. Max flow, MST algorithms) we return vectors of pairs of strings for the edges, why don't we return vectors of pointers to edges instead? I general, I feel that the interface is a bit too heterogeneous concerning algorithms returns type.

On the fact that we can return the pointer, i'm not sure is the correct way, the result can be modified and should not impact the original graph.

• With this choice of data structure (i.e. std::vector) it is kind of hard to get the strongly connected component for a given vertex (linear time search). Maybe doing something like what has been done for DialResult (i.e. unordered_map) could simplify this. And providing hashing for single vertices is not really a problem since they already have an id attribute.

This second opportuniy can be evalueted in a new issue, maybe an unordered_map can be a good solution.