HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Random access iterator for custom data type in C++

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomiteratortypecustomfordataaccess

Problem

I am working with an array-like data container through a 3rd party API. So I can not change the container itself to add iterators. There are many different array-like classes, and each have a specific data type it will store. There are two unknowns in this case, the class of the array container and the data type stored in the container. Though all of them have the same interface by getting a value by index - operator[]. So I wrote a random access iterator to wrap around these classes so I don't have to specialize or define the container class and data return types.

I would like some feedback on my code and any suggestions for improvements or errors. I have tested this and it works as expected so far.

```
template
class MayaArrayRange {
protected:
typedef typename std::remove_reference()[0])>::type item_type;

public:
template
class MayaArrayIter : public std::iterator {
friend class MayaArrayRange;

protected:
C* c;
unsigned int i;

MayaArrayIter(C* c) : c(c), i(0) {}
MayaArrayIter(C* c, unsigned int i) : c(c), i(i) {}

public:
typedef typename std::iterator::pointer pointer;
typedef typename std::iterator::reference reference;
typedef typename std::iterator::difference_type difference_type;

template
MayaArrayIter(const MayaArrayIter& other) : c(other.c), i(other.i) {}

template
MayaArrayIter& operator=(const MayaArrayIter& other) {
c = other.c;
i = other.i;
return *this;
}

reference operator*() const {
return (*c)[i];
}

pointer operator->() const {
return &(*c)[i];
}

MayaArrayIter& operator++() {
++i;
return *this;
}

MayaArrayIter& operator--() {
--i;
return *this;
}

MayaArrayIter operator++(int) {
return MayaArrayIter(c, i++);
}

Ma

Solution

The code looks good, and I see nothing to suggest that it wouldn't work well - it's just that there's a lot of code that doesn't need to exist. So here's a review full of suggested deletions.

Unnecessary Typedefs

Inheriting from std::iterator already provides you with the pointer, reference, and difference_type typenames. That's the entire point of inheriting from std::iterator. You do not need to provide them yourself.

Unnecessary index

Your MayaArrayIter contains both a C* and an unsigned int. Why? You don't need both. Just take a pointer that already points to the correct index. Constructing a MayaArrayIterator(cnt, 5) would act equivalently to MayaArrayIterator(&cnt[0] + 5), so let's just do that.

Extend that out to all the member functions.

Unnecessary functions

MayaArrayIter is a simple pod, so the compiler-generated defaults for the copy constructor and copy assignment already do the right thing - you do not need to reimplement them. If you want to write them out, you should = default them.

Unnecessary Second Template Argument

Once we fix MayaArrayIter to just hold a pointer to the data, it effectively just holds a V*. So we just need the one template argument.

Unnecessary Pointer Prefer a Reference

Had to break the theme here, ugh. Anyway, if MayaArrayRange just exists to wrap a random access range - you should keep a reference to the container (and name it something more meaningful than a) rather than a pointer.

Even better would be to make the range container-agnostic so that a MayaArrayRange can either refer to an int[30] or a std::array or a std::vector or ...

Context

StackExchange Code Review Q#113957, answer score: 5

Revisions (0)

No revisions yet.