Custom C++ iterator

Question

So I have written a custom iterator for std::map that allows access to just the "values" of the map and I was wondering if it is the "correct" way to implement std::iterator (acts identically to std::iterator).

Here is the implementation I have written:

template<typename K, typename V>
class MapValueIterator : public std::iterator<std::forward_iterator_tag, V> {
public:
    MapValueIterator();

    MapValueIterator(typename std::map<K, V>::iterator iterator) : iterator(iterator) { }

    MapValueIterator(const MapValueIterator<K, V> &iterator) : iterator(iterator.iterator) { }

    MapValueIterator<K, V> &operator=(const MapValueIterator<K, V> &iterator) {
        this->iterator = iterator.iterator;
    }

    bool operator==(const MapValueIterator<K, V> &iterator) const {
        return this->iterator == iterator.iterator;
    }

    bool operator!=(const MapValueIterator<K, V> &iterator) const {
        return this->iterator != iterator.iterator;
    }

    V &operator*() const {
        return (iterator->second);
    }

    V *operator->() const {
        return &(iterator->second);
    }

    MapValueIterator<K, V> &operator++() {
        ++iterator;

        return *this;
    }

    MapValueIterator<K, V> operator++(int) {
        return (*this)++;
    }

private:
    typename std::map<K, V>::iterator iterator;
};

And an example of how it can be used:

std::map<int, int> values;

values.insert(std::make_pair(0, 1));
values.insert(std::make_pair(1, 2));
values.insert(std::make_pair(2, 5));

MapValueIterator<int, int> begin(values.begin());
MapValueIterator<int, int> end(values.end());

for (auto iterator = begin; iterator != end; ++iterator) {
    printf("%d\n", *iterator);
}

Answer 1

It looks about perfect.

You got the post increment wrong though:

MapValueIterator<K, V> operator++(int) {
    return (*this)++;
}

I believe this goes into an infinite recursion.

Thus this is normally implemented as:

MapValueIterator<K, V> operator++(int) {
    MapValueIterator<K, V>  result(*this);  // get a copy for return
                                            // so this can be used
                                            // unaltered in the expression

    // Now implement the current object.
    ++(*this);

    // Returned the saved copy.
    return result;
}

The only difference is that the std::map<K,V>::iterator type actually implements the Bidirectional iterator concept. The question is why is your iterator only implement the Forward iterator concept.

Answer 2

I have one other addition to what Loki has already pointed out.

The C++ committee has voted to deprecate inheriting from std::iterator as of C++17. The current recommendation is apparently to write those typedefs yourself.

template<typename K, typename V>
class MapValueIterator {
public:
    typedef V value_type;
    typedef std::ptrdiff_t difference_type;
    typedef V *pointer;
    typedef V &reference;
    typedef std::ForwardIteratorTag iterator_category;

// ...

Feel free to read the proposal to deprecate it, if you care about why they're doing it (and such).