Fix circle layout docs and cleanup#469
Conversation
|
any feedback on this tiny fix @jeremy-murphy |
|
|
||
| vertices_size_type i = 0; | ||
| double two_pi_over_n = 2. * pi / n; | ||
| auto i = 0; |
There was a problem hiding this comment.
I believe auto i = 0 is a slight regression on type-correctness. It deduces int, where the original code used the graph's vertices_size_type On graphs with more than INT_MAX vertices this would overflow ?
With vertices_size_type, ++i cannot overflow because i only reaches n - 1, and n came from num_vertices(g) that returns vertices_size_type. By construction, i stays within the representable range of its type.
There was a problem hiding this comment.
reverted to the vertices_size_type
Becheler
left a comment
Becheler
left a comment
There was a problem hiding this comment.
Let's see what @jeremy-murphy thinks but I would be ok merging this PR if the type-correctness issue is confirmed and fixed ? Maybe that would warrant a comment ?
2 participants
Fix synopsis in the circle_layout docs (currently broken)
Incidentally I looked at the source code of that function and decided to do some small tweaking of the source code to add a const, use auto and remove a typedef.