What is the right way to implement an iterator that iterates over a Recordset provided below in C style?

class Recordset
{
public:

    Recordset(const Recordset&) = delete;

    Recordset& operator = (const Recordset&) = delete;

    Recordset(Recordset&& other) noexcept = default;

    Recordset& operator = (Recordset&&) = default;

    //Moves to the next record. Returns false if the end is reached.
    bool Next();

    //Gets the current record as an instance of type T.
    template <class T>
    void Get(T& val);
};

my idea is that I probably do something like this:

template <class T>
class Iterator
{
public:

    using iterator_category = std::forward_iterator_tag;

    using value_type = T;

    using difference_type = std::ptrdiff_t;

    using pointer = value_type*;

    using reference = value_type&;

    Iterator() = default;
    
    Iterator(Recordset s) : m_i(std::move(s))
    {
        try_next();
    }

    Iterator(const Iterator&) = delete;

    Iterator& operator = (const Iterator&) = delete;

    Iterator(Iterator&& other) = default;

    Iterator& operator = (Iterator&& other) = default;

    T* operator-> () { return cur(); }

    T* operator* () { return cur(); }

    bool operator== (const Iterator& other) const noexcept
    {
        //They both are end().
        return !m_v && !other.m_v;
    }

    bool operator!= (const Iterator& other) const noexcept
    {
        return !operator==(other);
    }

    Iterator& operator   ()
    {
        this->try_next();

        return *this;
    }

    Iterator operator   (int)
    {
        Iterator tmp = *this; //would not compile.

        this->try_next();

        return tmp;
    }

private:

    bool try_next()
    {
        if (m_i.Next())
        {
            T val;
            m_i.Get(val);

            m_v = val;

            return true;
        }

        return false;
    }

    T* cur()
    {
        T& val = *m_v;

        return &val;
    }

    Recordset m_i;

    std::optional<T> m_v;
};

template <class T>
std::ranges::subrange<Iterator<T>> make_range(Recordset& s)
{
    return std::ranges::subrange(Iterator<T>(s), Iterator<T>{});
}

and use it as follows:

struct Record { int x; std::string y; };

int main()
{
    Recordset s;

    for (Record& r : make_range(s))
    {
        std::cout << r.x << r.y << std::endl;
    }

    return 0;
}

The frist question is how do I implement Iterator operator (int) if both Recordset and Iterator are move-only? (temp and this can't point to different records, because there is only one current record in the recordset). Does C 20 require it?

The second question is it a good idea to implement end() in this way? (end() is a simply an iterator containing an empty optional)

CodePudding user response：

Single pass move-only input iterators (A c 20 std::input_iterator) are only required to be weakly incremental, where (void) i has the same effect as (void) i . You can simply have void operator (int) { *this; }. Older requirements for iterators (Cpp17InputIterator) requires iterators to be copyable, and require operator to return that copy.

And for your second question, you might want to use a sentinel type, something like:

inline constexpr struct {} Sentinel;

template<typename T>
bool operator==(const Iterator<T>& it, decltype(Sentinel)) {
    return !it.m_v;
}

// != can be rewritten from ==, so no need to write one

template <class T>
auto make_range(Recordset& s)
{
    return std::ranges::subrange(Iterator<T>(s), Sentinel);
}

And if you need to work with a algorithm that can't use separate sentinel types, use ranges::common_view. Your current solution also works, except you need to have this == &other || (!m_v && !other.m_v);.