I can not figure out how to elegantly combine the take with action of removing the taken elements from container.

Consider the following code:

static constexpr int kBatchSize = 4;
void fn(std::deque<int>& vals) {
    const auto elems = std::views::take(vals, kBatchSize);
    fmt::print("\n[");
    for (const auto& elem : elems) {
        fmt::print("{} ", elem);
    }
    fmt::print("]\n");

    for ([[maybe_unused]] const auto&_:elems) {
        vals.pop_front();
    }
}

It works fine(AFAIK) but I need to have an extra iteration since I can not figure out a way to combine taking of elements and removal.

note: fmt::print is just an example of logic per element, please do not focus on the fact that this could be done in a nicer way. In real code function is not a simple print.

Is there a elegant way to combine this into a single pass?

CodePudding user response：

You can do this without modifying vals. By converting the vals into a subrange and assigning each time to the subrange, for example:

#include <deque>
#include <ranges>
#include <fmt/ranges.h>

static constexpr int kBatchSize = 4;
auto fn(std::ranges::view auto vals) {
  const auto elems = std::views::take(vals, kBatchSize);
  fmt::print("{}\n", elems);

  return vals | std::views::drop(kBatchSize);
}

int main() {
  std::deque vals{1,2,3,4,5,6};
  auto res = fn(std::ranges::subrange{vals}); // [1, 2, 3, 4]
  res = fn(res); // [5, 6]
  res = fn(res); // []
}

Demo

The reason you can do this is that std::views::take and std::views::drop have specializations for subranges thanks to P1739, i.e. they return a subrange of the correct size.

CodePudding user response：

If you were using Range-v3, you could loop on the views::take(k) and then actions::drop(k) afterwards, like this:

#include "fmt/format.h"

#include <iostream>
#include <range/v3/action/drop.hpp>
#include <chrono>
#include <vector>
#include <deque>
#include <ranges>

using ranges::actions::drop;
using std::ranges::views::take;

static constexpr int kBatchSize = 4;
void fn(std::deque<int>& vals) {
    fmt::print("\n[");
    for (const auto& elem : vals | take(kBatchSize)) {
        fmt::print("{} ", elem);
    }
    fmt::print("]\n");
    drop(vals, 4);
}

int main() {
    std::deque vals{1,2,3,4,5,6};
    fn(vals);
    fn(vals);
}

This doesn't really free you from having

an extra iteration

but at least it hides it behind ranges::actions::drop, which tells very clearly what it does, given its name (drops the elements) and the usage (mutates the input otherwise without smth = on its left it would be a no-op).

---

If you want to

1. eventually mutate the vals parameter by removing some its leading elements,
2. but you also want do to something with those elements,

it's clear that you have to do 1 before 2, because those elements get destroyed, so you either

• operate on the elements and delete them one at a time,
• or you operate on all of them and then delete all of them.

The latter approach seems to be more straightforward and it's the one used by the proposed solutions (at least mine and Nicol Bolas'), which means you loop twice on those first elements. The former approach would be more complicated and just loop once instead of twice, which is not a big deal, I think.

Anyway, this is a solution which implements the first approach, looping only once:

#include "fmt/format.h"

#include <range/v3/algorithm/for_each.hpp>
#include <range/v3/view/iota.hpp>
// the other include directives

using namespace ranges;
using namespace ranges::views;

static constexpr int kBatchSize = 4;
void fn(std::deque<int>& vals) {
    fmt::print("\n[");
    for_each(
        iota(0, std::min(kBatchSize, (int)vals.size())),
        [&vals](auto){
            fmt::print("{} ", vals.front());
            vals.pop_front();
        }
    );
    fmt::print("]\n");
}

However, consider that dropping elements one at a time with .pop_front() can be expensive for other containers (e.g. vector doesn't have it, but if it did, it would very expensive (and that's the reason why it hasn't such a member function)).

Beside this performance consideration, there's also another problem. To highlight it, let's if we can abstract out what you called apply_drop:

void apply_drop(auto& vals, int n, auto f) {
    for_each(
        iota(0, std::min(n, (int)vals.size())),
        [&vals, f](auto){
            f(vals.front());
            vals.pop_front();
        }
    );
}

This does mostly what you meant, I hope. And fn would be implemented in terms of it like this:

void fn(std::deque<int>& vals) {
    fmt::print("\n[");
    apply_drop(vals, kBatchSize, [](auto const& x){ fmt::print("{} ", x); });
    fmt::print("]\n");
}

But as you see I had to keep a few print statements in fn and couldn't bring them in apply_drop, because doing so would make no sense. Indeed, when in your comment you write

provides a callback customization point.

I think you're referring to a callback function that is called for each element which you pop. And that cuts out the actions fmt::print("\n["); and fmt::print("]\n");, which have to be performed one off each, and in different moments; fmt::print("{} ", x); is the only thing that you are doing once per value, and so that's the only action that can be injected in the function that loops on the values (apply_drop in our case).

So your fn is not really simply "removing some leading elments and performing an action on each" (that's what apply_drop above does), but it's also doing other stuff that's not easy to shoehorn in the into for_each or another looping abstraction.

CodePudding user response：

Views cannot modify containers; their job is to be a view of a range.

If you just want to iterate through the first X elements and then delete all of them (unless you want to copy those elements, it kinda has to be in that order), that's easily done with std::for_each_n:

void fn(std::deque<int>& vals)
{
    fmt::print("\n[");
    auto end_it = std::for_each_n(vals.begin(), kBatchSize, [](int const& elem){
        fmt::print("{} ", elem);
    });
    fmt::print("]\n");

    vals.erase(vals.begin(), end_it);
}