Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor functions to accept a range instead of a pair of iterators #1090

Open
kkarbowiak opened this issue Apr 17, 2024 · 2 comments
Open
Assignees
Labels
Milestone

Comments

@kkarbowiak
Copy link
Contributor

There are multiple functions that accept a pair of iterators to denote a range, which could be refactored to accept a range (or a view). This could help raise the abstraction level a bit and make the code more readable.

In particular, is seems the following overloads

template void TWRoute::replace(const Input& input,
const Amount& delivery,
const std::vector<Index>::iterator first_job,
const std::vector<Index>::iterator last_job,
const Index first_rank,
const Index last_rank);
template void
TWRoute::replace(const Input& input,
const Amount& delivery,
const std::vector<Index>::const_iterator first_job,
const std::vector<Index>::const_iterator last_job,
const Index first_rank,
const Index last_rank);
template void
TWRoute::replace(const Input& input,
const Amount& delivery,
const std::vector<Index>::reverse_iterator first_job,
const std::vector<Index>::reverse_iterator last_job,
const Index first_rank,
const Index last_rank);
template void
TWRoute::replace(const Input& input,
const Amount& delivery,
const std::array<Index, 1>::const_iterator first_job,
const std::array<Index, 1>::const_iterator last_job,
const Index first_rank,
const Index last_rank);

could be replaced with a single one based on a view. Even the somewhat artificial code that wraps an element in a single-item-array could be replaced with a single-element-view (std::ranges::views::single).

@jcoupey
Copy link
Collaborator

jcoupey commented Apr 25, 2024

I'm all for it: that is something I've had in mind since I first heard about ranges before they even were part of the standard! Never got around to doing it. @kkarbowiak do you want to give this a go?

@kkarbowiak
Copy link
Contributor Author

I'm all for it: that is something I've had in mind since I first heard about ranges before they even were part of the standard! Never got around to doing it. @kkarbowiak do you want to give this a go?

Yes :-)

@jcoupey jcoupey added this to the v1.15.0 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants