feat: Shortest path for weighted graph

[joweich]

This is meant to add Dijkstra's algorithm for calculating the shortest path between two vertices in a weighted graph.

[codecov]

Codecov Report

Patch coverage: 98.95% and project coverage change: -0.10% ⚠️

Comparison is base (82b853a) 99.87% compared to head (f99fe0a) 99.78%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #38      +/-   ##
==========================================
- Coverage   99.87%   99.78%   -0.10%     
==========================================
  Files          20       20              
  Lines         821      913      +92     
==========================================
+ Hits          820      911      +91     
- Misses          1        2       +1     

Files Changed	Coverage Δ
include/graaflib/algorithm/shortest_path.h	100.00% <ø> (ø)
include/graaflib/algorithm/shortest_path.tpp	96.96% <97.29%> (+0.09%)	⬆️
test/graaflib/algorithm/shortest_path_test.cpp	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bobluppes]

Thanks for working on this!

I have recently changed the CI to a matrix build such that we ensure the code builds with both gcc as well as clang. Sorry for the inconvenience 😬

I can do a more in-depth review later, but the overall approach looks good! I mostly looked into the build issues. Let me know if you continue to have issues there.

[bobluppes]

Hi, sorry for the delay, I finally managed to take a proper look at this.

It turns out the compilation issue was due to the return type of the get_shortest_path function. Originally this was defined as std::optional<GraphPath<E>>, where E is given by graph<V, E, S>. However, since this can now be an arbitrary edge class which derives from weighted_edge, we want this to be std::optional<GraphPath<WEIGHT_T>> instead. See the diff in be39a50 for more details.

I also made the test suite name globally unique in a0a60d1 (apparently gtest requires this).

Finally, I found a bug in the getters of a const graph. Calling a getter on the const class resulted in a stack overflow due to an infinite recursion 😅. Quite sneaky and we did not have a test for it. Fixed it on main and merged it into your branch.

Thanks for working on this! The code looks good, if you are up for adding a few tests codecov has flagged a few lines as not covered. Other than that looks good to go 👍🏻

[joweich]

@bobluppes This is ready to be merged from my end. The missing coverage for one added line seems to be a bug/configuration issue concerning codecov.
In a follow-up PR, we should think about merging the reconstruct_path function in the shortest path logics for weighted and unweighted graphs to reduce code duplication.

[bobluppes]

Awesome, thanks for working on this!

Congrats on the third great feature added to Graaf 🥳