Preprocessing iterator

After tweaking semantics a lot, it ended up being a very poor object version of std::transform<>() that can be used with other std algorithms.

Basically the iterator wraps a provided iterator, creating an output iterator and applies supplied functor when dereferenced and assigned to. The caveat is that users will need to identify parameter type themselves, which can be error prone, but might open opportunities for other usages. Input type should not be cv qualified, because it has overloads for both const lvalue reference and rvalue reference. If the user wishes to take parameter by value, they would need to write another functor for that.

```
#ifndef SUNRISE_PREPROCESSING_ITERATOR_HPP
#define SUNRISE_PREPROCESSING_ITERATOR_HPP

#include

namespace shino
{
template
class preprocessing_iterator :
public std::iterator
{
Functor functor;

Iterator iterator;
public:
preprocessing_iterator(const Functor &f, const Iterator &it) :
functor(f),
iterator(it)
{}

preprocessing_iterator(Functor &&f, Iterator &&it) :
functor(f),
iterator(it)
{}

class proxy
{
friend class preprocessing_iterator;

Iterator &iterator;
Functor &f;
public:
using value_type = InputType;

proxy &operator=(const value_type &value)
{
*iterator = f(value);
return *this;
}

proxy &operator=(value_type &&value)
{
*iterator = f(value);
return *this;
}

private:
proxy(Iterator &it, Functor &functor) :
iterator(it),
f(functor)
{}
};

proxy operator*()
{
return proxy(iterator, functor);
}

preprocessing_iterator &operator++()
{

If I understand correctly, your goal is to make this example code:

shino::preprocessing_iterator::iterator>
    conversion_iterator(string_to_int, output.begin());

std::copy(v.begin(), v.end(), conversion_iterator);

have an effect equivalent to this present-day code:

std::transform(v.begin(), v.end(), output.begin(), string_to_int);

(When I say it that way, it seems almost a little silly, doesn't it? ;))

The first usability improvement I'd make is to notice that your iterator type is fundamentally related to std::transform (actually you already noticed this) and change its name to shino::transform_iterator.

It also looks analogous to std::back_insert_iterator, in the sense that it holds references to some other objects and its constructor is really awkward to type out by hand. So I'd follow the practice of the STL here by creating a helper function. The logical name would be shino::make_transform_iterator, but you could instead follow traditional practice (cf. back_inserter) and call it simply std::transformer. It looks like this:

template
auto transformer(F f, OutputIterator dest)
    -> transform_iterator
{
    return {std::move(f), std::move(dest)};
}

So now your target example code looks like this:

auto conversion_iterator = shino::transformer(string_to_int, output.begin());
std::copy(v.begin(), v.end(), conversion_iterator);

That's an improvement!

Next, I notice that class InputType feels out-of-place. We don't actually use it for anything. So I'd try to get rid of it, by hunting down each place that your code depends on InputType and rewriting it to not use InputType.

For example, instead of

proxy &operator=(const value_type &value)
{
    *iterator = f(value);
    return *this;
}

proxy &operator=(value_type &&value)
{
    *iterator = f(value);
    return *this;
}

I would try to write simply

template
proxy& operator=(U&& value)
{
    *iterator = f(std::forward(value));
    return *this;
}

I was a little worried that this might do the wrong thing when chaining transform_iterators together, but my first trivial test case worked, so I'm going to ignore that nagging worry. :) That test case was:

auto p1 = shino::transformer(string_to_int, output.begin());
auto p2 = shino::transformer([](std::string x){ return x+"1"; }, p1);
std::copy(v.begin(), v.end(), p2);

The other place where you use InputType is in your inheritance from std::iterator. Fortunately, inheritance from std::iterator is no longer considered à la mode; in fact, std::iterator itself may become deprecated soon! So what you should do is simply

template
class transform_iterator
{
    Functor functor;
    Iterator iterator;

  public:
    using iterator_category = std::output_iterator_tag;
    using difference_type = void;
    using value_type = void;
    using pointer = void;
    using reference = void;

Notice that std::iterator_traits strongly encourages you to put the using value_type = void; line; and pointer, reference, and difference_type also become void by analogy to std::back_insert_iterator.

Notice that these member typedefs must be public. (libc++ is happy if they're private; but libstdc++ isn't.)

Lastly, your specialization of std::swap is also no longer à la mode. Instead of interjecting your own code into the std namespace, you should simply provide a shino::swap(transform_iterator&, transform_iterator&) that can be found by ADL.

Your swap code itself is 100% correct and reasonable; you should just change that namespace std to namespace shino, and you'll be in business.

The expected way for users (such as STL algorithms) to swap two transform_iterators a and b is:

using std::swap;  // in case no ADL swap exists
swap(a, b);  // look for an ADL swap; fall back on std::swap if needed

I have no particular comment on the efficiency or subtle correctness of how your code (or my revised code) handles the forwarding of functors and such. I'd probably just try to write some tests for that (e.g. what if the functor in question is move-only?), and fix any issues they find, and then move on with my life. ;)

Here's a Wandbox of the code after these changes.

StackExchange Code Review Q#155866, answer score: 6