CODEWITHSUNDEEP
c++visual-c++c++17
Zoi Lisoi
Zoi Lisoi

Reputation: 3

My iterator implementation don't work, what's wrong with it?

I try to write my implementation of c++ iterator, but met some problems. When I try to iterate in range-based for, but get error that my iterator don't have suitable "begin" function and none was found. I wrote begin() and end() function. What's wrong?

#include <iostream>
template<typename T>
class iter final {
private:
    T* current_ = nullptr;
    T* end_ = nullptr;
    int n;
public:
    iter(T* first) :current_(first) {};
    iter() {};
    inline T& operator+ (int n) { return *(current_ - n); }
    inline T& operator- (int n) { return *(current_ - n); }

    inline T& operator++ (int) { return *current_++; }
    inline T& operator-- (int) { return *current_--; }
    inline T& operator++ () { return *++current_; }
    inline T& operator-- () { return *current_--; }

    inline bool operator!= (const iter& it) { return current_ != it.current_; }
    inline bool operator== (const iter& it) { return current_ == it.current_; }

    inline T& operator* () { return *current_; }

    inline iter<T>* begin() { return current_; }
    inline iter<T>* end() { return end_ + n; }
    inline void set_current(T* _current) { current_ = _current; }
    inline void set_end(T* _end) { end_ = _end; n = (end_ - current_) / sizeof(_end); }
    inline void set_num(int num) { n = num; }
};
template<typename T>
class shuffle_range final {
public:
    shuffle_range() { it = new iter<T>; };
    ~shuffle_range() { delete it; };
    iter<T>* it;
};
template<typename T>
iter<T>* make_shuffle(T* begin, T* end) {
    static shuffle_range<T> shuffler;

    shuffler.it->set_current(begin);
    shuffler.it->set_end(end);
    shuffler.it->set_num(end - begin);
    return shuffler.it;
}

int main() {
    int a[] = { 0, 4, 5, 2, 8, 1 };
    auto shuffle_a = make_shuffle(a, (a + 5));
    for (auto i : shuffle_a)
        std::cout << i << " ";
    return 0;
}

Upvotes: 0

Views: 95

Answers (2)

Coral Kashri
Coral Kashri

Reputation: 3506

For your question - make_shuffle returns a pointer to iter<T> class, which means that in order to iterate over this instance, you need to get the pointer value:

for (auto i : *shuffle_a)
    std::cout << i << " ";

However, pay attention that you have another issue with your end function implementation. It checks for end_ + n instead of end_ or current_ + n. My suggestion is to get rid from one of them (select which one you use), and use a function to get the other one. For example, you can add size() member function to get n:

inline size_t size() { return (end_ - current_) / sizeof(end_); }

Upvotes: 1

kesarling
kesarling

Reputation: 2216

A for each loop in C++ is just a masking of the normal for loop which goes from start to end of a given list of objects of the same type. It even gets converted to one. So, in you case, the compiler does not know how to process the for-each loop. It would be better if you instead used a normal for loop. Something like:

for(auto i = shuffle_a.begin(); i != shuffle_a.end(); i++) { //mind well!!! `end()` must return an iterator pointing to the element after the last element
    <whatever>
}

will suffice

Upvotes: 1

Related Questions