std::ranges zip view to iterate over two equal size ranges at the same time

Question

Since c++20 std::ranges doesn't have a zip view like range-v3 does, I am working on implementing a basic version of such a zip view to iterate over two ranges at the same time. The ranges should be equal in size. If they are not equal in size, the view should iterate over both ranges and stop as soon as one range has reached the end.

I would like feedback on whether this implementation is correct (iterator implementation, references, forwarding) and if it could be improved in any way.

Demo on Compiler Explorer: https://godbolt.org/z/r7Y8hfcd8

My Implementation:

#include <concepts>
#include <ranges>
#include <vector>

template<std::ranges::input_range Rng1, std::ranges::input_range Rng2>
    requires std::ranges::view<Rng1> && std::ranges::view<Rng2>
struct zip_view : std::ranges::view_interface<zip_view<Rng1, Rng2>>
{
    using reference = std::pair<
        std::ranges::range_reference_t<Rng1>,
        std::ranges::range_reference_t<Rng2>>;
    using const_reference = std::pair<
        std::ranges::range_reference_t<Rng1>,
        std::ranges::range_reference_t<Rng2>>;
    using value_type = std::pair<
        std::remove_cv_t<
            std::remove_reference_t<std::ranges::range_reference_t<Rng1>>>,
        std::remove_cv_t<
            std::remove_reference_t<std::ranges::range_reference_t<Rng2>>>>;

    struct iterator;
    using const_iterator = iterator;

private:
    std::ranges::iterator_t<Rng1> m_rng1_begin, m_rng1_end;
    std::ranges::iterator_t<Rng2> m_rng2_begin, m_rng2_end;

public:
    zip_view() = default;
    zip_view(Rng1&& rng1, Rng2&& rng2)
        : m_rng1_begin(std::begin(rng1)),
          m_rng1_end(std::end(rng1)),
          m_rng2_begin(std::begin(rng2)),
          m_rng2_end(std::end(rng2))
    {
    }

    zip_view(Rng1 const& rng1, Rng2 const& rng2)
        : m_rng1_begin(std::begin(rng1)),
          m_rng1_end(std::end(rng1)),
          m_rng2_begin(std::begin(rng2)),
          m_rng2_end(std::end(rng2))
    {
    }

    iterator begin() const { return iterator(m_rng1_begin, m_rng2_begin); }
    iterator end() const { return iterator(m_rng1_end, m_rng2_end); }

    const_iterator cbegin() const
    {
        return const_iterator(m_rng1_begin, m_rng2_begin);
    }
    const_iterator cend() const
    {
        return const_iterator(m_rng1_end, m_rng2_end);
    }
};

template<typename Rng1, typename Rng2>
zip_view(
    Rng1&& rng1, Rng2&& rng2
) -> zip_view<std::ranges::views::all_t<Rng1>, std::ranges::views::all_t<Rng2>>;

template<std::ranges::input_range Rng1, std::ranges::input_range Rng2>
    requires std::ranges::view<Rng1> && std::ranges::view<Rng2>
struct zip_view<Rng1, Rng2>::iterator
{
    using difference_type = ptrdiff_t;
    using value_type = zip_view<Rng1, Rng2>::value_type;

private:
    std::ranges::iterator_t<Rng1> m_it1;
    std::ranges::iterator_t<Rng2> m_it2;

public:
    iterator() = default;
    iterator(
        std::ranges::iterator_t<Rng1> it1, std::ranges::iterator_t<Rng2> it2
    )
        : m_it1(std::move(it1)), m_it2(std::move(it2))
    {
    }

    iterator& operator++()
    {
        ++m_it1, ++m_it2;
        return (*this);
    }

    iterator operator++(int) { return ++iterator(m_it1, m_it2); }

    bool operator==(iterator const& other) const
    {
        // check if one of the iterators is equal so we don't iterate past the
        // end in case (m_it1 == end) and (m_it2 != end) or (m_it1 != end) and
        // (m_it2 == end)
        return (m_it1 == other.m_it1) || (m_it2 == other.m_it2);
    }

    bool operator!=(iterator const& other) const
    {
        // check if both of the iterators are unqequal so we don't iterate past
        // the end in case (m_it1 == end) and (m_it2 != end) or (m_it1 != end)
        // and (m_it2 == end)
        return (m_it1 != other.m_it1) && (m_it2 != other.m_it2);
    }

    reference operator*() { return reference(*m_it1, *m_it2); }

    const_reference operator*() const
    {
        return const_reference(*m_it1, *m_it2);
    }

    reference operator->() { return reference(*m_it1, *m_it2); }

    const_reference operator->() const { return reference(*m_it1, *m_it2); }
};

static_assert((bool)std::input_iterator<zip_view<
                  std::views::all_t<std::vector<int>>,
                  std::views::all_t<std::vector<int>>>::iterator>);

static_assert((bool)std::ranges::input_range<zip_view<
                  std::views::all_t<std::vector<int>>,
                  std::views::all_t<std::vector<int>>>>);

static_assert((bool)std::ranges::view<zip_view<
                  std::views::all_t<std::vector<int>>,
                  std::views::all_t<std::vector<int>>>>);

Example Usage:

#include <vector>
#include <iostream>
#include <string>

int main()
{
    std::vector<int> v1({ 1, 2, 3, 4 });
    std::vector<std::string> v2({ "a", "b", "c", "d" });

    for (auto&& [a, b] : zip_view(v1, v2))
        std::cout << a << " : " << b << "\n";
}

Answer 1

The problem with returning a pair of references

It looks like you have a const_iterator which when dereferenced returns a const_reference, which sounds like it should be a pair of const references, but it isn't. And even if const pair of references is deduced somewhere, that is not the same as a pair of const references. So you can write this code:

const auto const_zipped_view = zip_view(v1, v2);

for (const auto& [a, b] : const_zipped_view) {
    a = -1;
    b = "oops!";
}

for (auto&& [a, b] : zip_view(v1, v2))
    std::cout << a << " : " << b << "\n";

And it will compile without any warnings and print "-1 : oops!" a few times. Only if v1 and/or v2 themselves are const will a pair of const references be deduced.

C++23 introduces std::ranges::range_const_reference_t, which might help create your const_reference, or you have to do it yourself somehow.

This also makes it clear you need a lot more unit tests to verify that all the assumptions you might have about your zipped view are actually correct.

Implement a real const_iterator

Your const_iterator is just the same as iterator, so if someone is explicitly calling cbegin() and cend() on a zip_view, they will still get a regular iterator that can potentially mutate the contents of the view.

About the constructor

I am uncertain what the proper way would be to write the constructor for a zip_view classes. However, I fail to see why you have one overload take rvalue-references, when you are not going to std::move() them. Just take them by lvalue-reference.

You can omit operator!=()

If you omit operator!=(), the compiler will generate a defaulted one that just calls operator==() and negates it, which is exactly what you want.

Make the iterator more flexible

Your iterator only implements the bare minimum to be an input iterator. So it can only iterator over the view once, cannot iterate backwards, and cannot be used to have random access to the view, even if the two ranges do support all that.

One use case for allowing random access would be to be able to sort one array based on the contents of another, by writing something like:

std::vector<Key> keys = {…};
std::vector<Value> values = {…};
std::ranges:sort(zip_view(keys, values), {}, [](auto& kvp){return kvp.first;});

Of course, this will require a lot more effort to implement.

Allow zipping more than two ranges at a time

It would be a nice exercise to create a zip_view that can handle three or more ranges at a time. This requires making it a variadic template, using std::tuple instead of std::pair, and parameter pack expansions. However, it will not require more lines of code than you already have.