Today, I will require your help to improve an iterator adapter. The goal of the adapter (I called it get_iterator) is to adapt std::map iterators for example: map iterators return std::pair instances, a get_iterator<0, std::map<...>::iterator> will provide an iterator which iterates through the keys while a get_iterator<1, std::map<...>::iterator> will provide an iterator which iterates through the values. Here is my implementation:

I don't provide operator==, operator< and their friends in this question since their implementation is trivial and mimicks this of std::reverse_iterator. Are there functions lacking in the interface or things to be improved in the implementation? Also, do you have any idea of a more suitable name for such an iterator adaptor?

\$\begingroup\$Still getting used to the new syntax for the return type. Not sure why you are using it here. The normal syntax should work and is much clearer (in my opinion).\$\endgroup\$
– Martin YorkJan 2 '14 at 20:34

\$\begingroup\$@LokiAstari I personally find it clearer: it allows to split the return type and the rest of the function in one glance. Moreover, it resembles the functions notation in type theory. Well, it also allows me to always use one and only one return type syntax everywhere.\$\endgroup\$
– MorwennJan 3 '14 at 14:57

\$\begingroup\$Remember you are not writing code for you to understand. You wrote it, thus you should quickly pick it up again. You really need to write code for the next person that is not you. As they have to maintain it.\$\endgroup\$
– Martin YorkJan 3 '14 at 22:07

\$\begingroup\$@LokiAstari Well, at least I try to be consistent when writing it. I don't see what's the big deal with this syntax; I do not see how the old syntax is clearer.\$\endgroup\$
– MorwennJan 3 '14 at 22:25

1

\$\begingroup\$If you're actually planning on using this in a project, I feel like I need to point out boost has a transform_iterator that's quite a bit more flexible.\$\endgroup\$
– YuushiJan 9 '14 at 23:54

The function object would have had another advantage had Boost allowed its transform_iterator to take advantage of the empty base optimization. Unfortunately, this is not the case right now, but using a function object may trigger a free improvement when Boost finally implements EBO in transform_iterator.

I would have loved to use ADL to find the most suitable get to use, but get always need an integer template parameter. And ADL is not triggered when a template parameter is provided by hand instead of being deduced.

Add a construction function

Let's assume that we still want to write a get_iterator by hand. While the integer template parameter is always provided by hand, the ability to deduce the Iterator template parameter is still something we want to have. Therefore, we will add a construction function:

Moreover, adding such a construction function is consistent with the standard library. After all, std::make_reverse_iterator was added to C++14 for consistency.

Miscellaneous tidbits

There are more things that could be improved, but they are more subtle and less interesting:

The check &other != this in operator= seems pretty useless. It is almost always a pessimization and I cannot think of a scenario where assigning base() to itself would go wrong.

operator++(int) and operator--(int) shoud return get_iterator instead of get_iterator&. Currently they return references to temporary variables, which is plain wrong. That proves that I didn't write proper tests.

More iterator-related functions could have been overloaded to support the requirements of a [RandomAccessIterator][5] when needed since nothing prevents it and supporting more iterators is better than support fewer. The functions that could have been added are operator+, operator-, operator+=, opperator-= and opeator[].

It would be better to put = default on its first declaration instead of separating the declaration and the definition of the default constructor. The rationale is that Iterator it{} can zero-initialize the iterator instead of dedault-initializing it if = default appears on the first declaration. I have no idea whether this can change something for any iterator, but initializing memory with zero cannot be worse than initializing it with garbage.