Skip to content

Conversation

@georgthegreat
Copy link

@georgthegreat georgthegreat commented Dec 21, 2024

The library is marked as C++14, hence it should be safe to replace the dependency with native syntax.

boost::graph::vertices returns std::pair which is not compatible with range-based for.
As changing this behavior would result in API break, only public part was changed and the dependency still remains.

@jeremy-murphy jeremy-murphy self-assigned this Dec 21, 2024
@jeremy-murphy
Copy link
Contributor

Thanks!

@jeremy-murphy
Copy link
Contributor

If there are any build failures, please investigate.

@georgthegreat
Copy link
Author

@jeremy-murphy, I had to reduce the amount of changes as it turns out that range-based-for is incompatible with vertices() method (it returns std::pair which is not a range in c++ terms, yet is accepted by BOOST_FOREACH).

So, publicpart is rewritten and boost/foreach is no longer included from neither include/ nor src/ directories.

Can we merge such (partial) improvement?

@jeremy-murphy
Copy link
Contributor

Whilst I think it's good to get rid of macros like BOOST_FOREACH, what I think is even better is to use algorithms in preference to for () loops. And it turns out that the implementation in Boost.Ranges does support boost::pair (and I presume std::pair) as a range. However, I don't want you to go overboard with refactoring right away, as many of these loops have large bodies, so I recommend picking the low-hanging fruit, the stuff that looks obviously like a find_if, etc.

@georgthegreat
Copy link
Author

range-based for is much clearer to me compared to huge functional-style algorithms.

As I have failed to remove this dependency, I am closing this PR.

@georgthegreat georgthegreat deleted the foreach branch January 2, 2025 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants