patterncppMinor
Random access iterator for custom data type in C++
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 -
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
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
Unnecessary index
Your
Extend that out to all the member functions.
Unnecessary functions
Unnecessary Second Template Argument
Once we fix
Unnecessary Pointer Prefer a Reference
Had to break the theme here, ugh. Anyway, if
Even better would be to make the range container-agnostic so that a
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.