3
\$\begingroup\$

My goal was to create an zip-like iterator to selectively iterator over a subset of arrays in a larger manager object, compare itself to any other zip-like iterator, but can be copy constructed from any zip-like iterator with the same template parameters. It's to be used like this:

#include "Iterator.h"

struct Position { float x, y; };
struct Velocity { float x, y; };
struct CollisionBox { int h, w; };

class ExampleManager
{
    static constexpr std::size_t COUNT = 100;
    std::tuple<Position[COUNT], Velocity[COUNT], CollisionBox[COUNT]> mComponents;

public:
    ExampleManager() = default;

    template<class... As>
    auto begin() -> Iterator<As...> {
        return Iterator<As...>(0, std::get<As[COUNT]>(mComponents)...);
    }

    auto end() -> Iterator<void> {
        return Iterator<void>(COUNT);
    }
};

int main()
{
    ExampleManager manager;

    //... initialize components in manager

    //Iterate over position and velocity, ignores CollisionBox
    for (auto iter = manager.begin<Position, Velocity>(); iter != manager.end(); ++iter) {
        Position& p_ref = iter.get<Position>();
        Velocity& v_ref = iter.get<Velocity>();

        p_ref.x += v_ref.x;
        p_ref.y += v_ref.y;
    }

    //Check each entity against every other entity for a collision, ignores Velocity
    for (auto iter1 = manager.begin<Position, CollisionBox>(); iter1 != manager.end(); ++iter1) {
        for (auto iter2 = iter1 + 1; iter2 != manager.end(); ++iter2) {
            Position& p1_ref      = iter1.get<Position>();
            CollisionBox& cb1_ref = iter1.get<CollisionBox>();
            Position& p2_ref      = iter2.get<Position>();
            CollisionBox& cb2_ref = iter2.get<CollisionBox>();

            check_collision(p1_ref, cb1_ref, p2_ref, cb2_ref);
        }
    }
}

My iterator contains a size_t for indexing and a tuple of pointers to the beginning address of the array, I have a template specialization for Iterator<void> so to avoid code duplication I put all operations for indexing in a BaseIterator class, it's essentially an integer wrapper.

IteratorBase.h

class IteratorBase
{
public:
    using size_type = std::size_t;

protected:
    size_type mIndex;

protected:
    explicit IteratorBase(size_type pIndex);

    IteratorBase(const IteratorBase& rhs)                    = default;
    IteratorBase(IteratorBase&& rhs)                         = default;
    auto operator=(const IteratorBase& rhs) -> IteratorBase& = default;
    auto operator=(IteratorBase&& rhs) -> IteratorBase&      = default;

public:
    ~IteratorBase() = default;

public:
    //... other operator overloads like operator--, operator++(int), etc

    auto operator++() -> IteratorBase&;
    auto operator+=(const IteratorBase& rhs) -> IteratorBase&;
    auto operator+=(size_type rhs)           -> IteratorBase&;

    auto operator==(const IteratorBase& rhs) const -> bool;
    auto operator!=(const IteratorBase& rhs) const -> bool;
};

Iterator.h

#include <tuple>
#include "IteratorBase.h"

template<class A, class... Bs>
class Iterator : public IteratorBase
{
private:
    using base_type  = IteratorBase;
    using tuple_type = std::tuple<A* const, Bs* const...>;
    using base_type::mIndex;
public:
    using size_type = base_type::size_type;

private:
    tuple_type mComponents;

public:
    Iterator(size_type pIndex, A* const pFirstComponent, Bs* const... pOtherComponents)
    : base_type(pIndex)
    , mComponents(pFirstComponent, pOtherComponents...) {}

    template<class C>
    auto get() -> C& { 
        return std::get<C* const>(mComponents)[mIndex]; 
    }
    template<class C>
    auto get() const -> const C& { 
        return const_cast<Iterator*>(this)->get<C>(); }
    };

template<>
class Iterator<void> : public IteratorBase
{
private:
    using base_type = IteratorBase;
    using base_type::size_type;

public:
    explicit Iterator(size_type pIndex) : base_type(pIndex) {}
};

template<class A, class... Bs>
auto operator+(Iterator<A, Bs...> lhs, const IteratorBase& rhs) -> Iterator<A, Bs...>
{
    lhs += rhs;
    return lhs;
}

template<class A, class... Bs>
auto operator+(Iterator<A, Bs...> lhs, typename Iterator<A, Bs...>::size_type rhs) -> Iterator<A, Bs...>
{
    lhs += rhs;
    return lhs;
}

I'm like feedback on a few things here:

  • I don't care about the ExampleManager, I wrote that quickly to try and demonstrate the bare-bones interface, although the begin and end functions in the actual manager are more or less the same.
  • Correctness: Operator overloading combined with inheritance can be confusing. Are there any hidden bugs or odd edge cases where these iterators would break? I't should be assumed the pointers inside the class are always valid, and wont go out of range.
  • Best Practices: Is Iterator<void> necessary, is it confusing? Are there any improvements to the interface I could make?
\$\endgroup\$
1
  • 1
    \$\begingroup\$ The implementation of IteratorBase is somewhat crucial for the review... Also, please check the indentation. And are you sure about the C++ version? \$\endgroup\$ Commented Aug 4, 2017 at 20:55

1 Answer 1

2
\$\begingroup\$

Correctness: Operator overloading combined with inheritance can be confusing. Are there any hidden bugs or odd edge cases where these iterators would break?

Yes. Your return type It will be the base class, not the class the user was actually incrementing. That is, the requirements specify e.g. that i++ have a return type of It. Given some specific using It = Iterator<⋯whatever⋯>; It p1, p2; then p2= p1++; will give a compiler error.

Your base class needs to use the CRTP to make those reusable functions.


I put all operations for indexing in a BaseIterator class, it's essentially an integer wrapper.

You should expose that as a usable counting iterator. So, your array-zipper uses your counting iterator, rather than in internal component.


Is Iterator<void> necessary

I don’t see why. The normal template class takes a list of types will be found in arrays (or other contiguous sequences). Why would you ever use void as one of them? You are only handling void in the first position, anyway. Iterator<char,void,int&> would have problems working, too.

Best practices: Are there any improvements to the interface I could make?

I don’t like how the const form of a function calls the non-const form with a const_cast.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.