tcbrindle / NanoRange

Range-based goodness for C++17
Boost Software License 1.0
358 stars 28 forks source link

Switch to new return types #94

Closed bstaletic closed 4 years ago

bstaletic commented 4 years ago

This commit changes the return types of some algorithms, according to the <algorithm> header synopsis on cppreference.

For the following algorithms I am actually not sure if what I'm returning is correct:

In all cases I'm not 100% sure I'm returning the right value. Also, I've done the bare minimum with the tests to get them to compile and run. That means that I'm only testing the O from the partial_sort_copy_result and only end() of subrange<I>, which is what used to be tested before my pull request.

tcbrindle commented 4 years ago

Am I reading this correctly? All of the new subrange<I> returning algorithms return {i, last} and partial_sort_copy returns {last, result_first + N}? These return values don't make much sense to me...

Reference: partial_sort_copy, partition and stable_partition and unique

Bear in mind that when the spec tells you to return "last", it really means "an iterator pointing to last", rather than the actual sentinel (see here, but note that the end iterator is in most cases computed as a side-effect of the algorithm so you don't actually have to call next).

bstaletic commented 4 years ago

Both Travis and Appveyor are green, thankfully.

tcbrindle commented 4 years ago

Travis CI was successful: https://travis-ci.org/github/tcbrindle/NanoRange/builds/700606337

Thanks very much @bstaletic, hope I wasn't too annoying with the changes :)

bstaletic commented 4 years ago

Woohoo! You weren't annoying at all. I've been on the reviewer's side of pull requests many times.