HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Transforming iterator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
transformingiteratorstackoverflow

Problem

The question is follow-up to preprocessing iterator.

Specification:


Given Functor functor (which is Callable) and Iterator iterator (which is OutputIterator), iterator is created that behaves the same as underlying iterator, except it applies a functor to the incoming data and then passes the result into underlying iterator. Input type of the Functor is not required to match value type of the Iterator, but the input should be only single parameter (even defaulted arguments are not allowed).

Code:

```
#ifndef SUNRISE_TRANSFORM_ITERATOR_HPP
#define SUNRISE_TRANSFORM_ITERATOR_HPP

#include

namespace shino
{
template
class transform_iterator :
public std::iterator
{
Functor functor;
Iterator iterator;
public:
transform_iterator(const Functor& f, const Iterator& it) :
functor(f),
iterator(it)
{}

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

class proxy
{
friend class transform_iterator;

Iterator &iterator;
Functor &f;
public:

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

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

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

transform_iterator &operator++()
{
++iterator;
return *this;
}

transform_iterator operator++(int)
{
auto copy = *this;
++iterator; //might exhibit different behavior sometimes
return copy;
}

const Iterator& internal_iterator() const
{
return ite

Solution

Lets start with the easy thing...

You are told to take any Callable (also class member pointer(!)) but you only use f(...), which doesn't work for all Callable types. To show you an example which should work according the problem description, but does not:

struct Person
{
    int age_;
    std::string name_;

    int age() const noexcept
    {
        return age_;
    }

    decltype(auto) name() const noexcept
    {
        return name_;
    }
};

int main()
{
    auto vec = std::vector(2);
    Person persons[] = { {24, "Foo"}, {42, "Bar"} };

    auto iter = shino::transform_iterator{&Person::age, vec.begin()};
    std::copy(std::begin(persons), std::end(persons), iter);

    // should print "24\n42\n"
    for (int x : vec) {
        std::cout << x << '\n';
    }
}


Error message:

shino.cpp:37:29: error: called object type 'int (Person::*)() const noexcept' is not a function or function pointer
                *iterator = f(std::forward(value));
                            ^
/usr/local/include/c++/v1/algorithm:1706:19: note: in instantiation of function template specialization 'shino::transform_iterator >::proxy::operator=' requested here
        *__result = *__first;
                  ^
/usr/local/include/c++/v1/algorithm:1731:19: note: in instantiation of function template specialization 'std::__1::__copy > >' requested here
    return _VSTD::__copy(__unwrap_iter(__first), __unwrap_iter(__last), __unwrap_iter(__result));
                  ^
shino.cpp:137:7: note: in instantiation of function template specialization 'std::__1::copy > >' requested here
        std::copy(std::begin(persons), std::end(persons), iter);
             ^
1 error generated.


So instead of the line

*iterator = f(std::forward(value));


you should use

*iterator = std::invoke(f, std::forward(value));


and the example works! (Proof) Lets tackle the next 'issue'

The problem description says specifically:


Given Functor functor (which is Callable) and Iterator iterator (which
is OutputIterator), [a transform_]iterator is created that behaves
the same as [the] underlying iterator

This could also mean in my opinion, that if you are given an OutputIterator you shall return one, but if you are given a ForwardIterator, which is just a refinement of OutputIterator, you shall also return a ForwardIterator and so on. And if this shall be production code, than I would also add the requirement, that your code shall not compile for given iterators which only satisfy InputIterator.

So to sum up, since you declare your transform_iterator to always be a an OutputIterator by setting that tag, you can not use it well with the (future) STL whenever it would make sense (well at least when we are going to have Concepts). You do not support BidrectionalIterator or RandomAccessIterator already.

To achieve better compliance with the STL, you could just copy the underlying iterator_tag (if it is not input_iterator_tag) and you could use std::enable_ifs to enable only proper operators. To solve this issue cleanly you have to make bigger changes.

You can look into the libraries ranges-v3 and cmcstl2 for some inspiration. They have fantastic iterator facades, which might go into the standard some day (Ranges-TS).

Next issue:
The following does not compile (on clang-5.0-trunk)

int main()
{
    auto vec = std::vector(10);

    auto iter = shino::transformer(
        [](auto i){ return i*i; },
        vec.begin()
    );
    auto end = shino::transformer(
        [](auto i){ return i*i; },
        vec.end()
    );

    std::iota(iter, end, 1);

    for (auto&& x : vec) {
        std::cout << x << '\n';
    }
}


Clang fails to compile, because the two closures have different types

Error message:

shino.cpp:127:2: error: no matching function for call to 'iota'
        std::iota(iter, end, 1);
        ^~~~~~~~~
/usr/local/include/c++/v1/numeric:196:1: note: candidate template ignored: deduced conflicting types for parameter '_ForwardIterator'
      ('transform_iterator' vs. 'transform_iterator')
iota(_ForwardIterator __first, _ForwardIterator __last, _Tp __value_)
^


Since iterator and sentinel can not be different in the current STL algorithms, there is unfortunately not much one can do right now. This will be different in the future. Until then I would suggest adding a function which maps an output range into a transformed output range (ensuring using the same Callable type).

Some more 'nitpicking':

As I already stated in the comments you could use std::decay_t when removing const-qualifiers and references from types. But as shown in my examples, you dont need make_functions anymore with recent compiler versions.

Edit:

I forgot to talk about making your Callable types Semiregular. The thing is, that Iterator types are required to be Regular, ie. DefaultConstructible and EqualityComparable. This means, that as long as your Functor type is not `Defau

Code Snippets

struct Person
{
    int age_;
    std::string name_;

    int age() const noexcept
    {
        return age_;
    }

    decltype(auto) name() const noexcept
    {
        return name_;
    }
};

int main()
{
    auto vec = std::vector<int>(2);
    Person persons[] = { {24, "Foo"}, {42, "Bar"} };

    auto iter = shino::transform_iterator{&Person::age, vec.begin()};
    std::copy(std::begin(persons), std::end(persons), iter);

    // should print "24\n42\n"
    for (int x : vec) {
        std::cout << x << '\n';
    }
}
shino.cpp:37:29: error: called object type 'int (Person::*)() const noexcept' is not a function or function pointer
                *iterator = f(std::forward<U>(value));
                            ^
/usr/local/include/c++/v1/algorithm:1706:19: note: in instantiation of function template specialization 'shino::transform_iterator<int (Person::*)() const noexcept,
      std::__1::__wrap_iter<int *> >::proxy::operator=<Person &>' requested here
        *__result = *__first;
                  ^
/usr/local/include/c++/v1/algorithm:1731:19: note: in instantiation of function template specialization 'std::__1::__copy<Person *, shino::transform_iterator<int (Person::*)()
      const noexcept, std::__1::__wrap_iter<int *> > >' requested here
    return _VSTD::__copy(__unwrap_iter(__first), __unwrap_iter(__last), __unwrap_iter(__result));
                  ^
shino.cpp:137:7: note: in instantiation of function template specialization 'std::__1::copy<Person *, shino::transform_iterator<int (Person::*)() const noexcept,
      std::__1::__wrap_iter<int *> > >' requested here
        std::copy(std::begin(persons), std::end(persons), iter);
             ^
1 error generated.
*iterator = f(std::forward<U>(value));
*iterator = std::invoke(f, std::forward<U>(value));
int main()
{
    auto vec = std::vector<int>(10);

    auto iter = shino::transformer(
        [](auto i){ return i*i; },
        vec.begin()
    );
    auto end = shino::transformer(
        [](auto i){ return i*i; },
        vec.end()
    );

    std::iota(iter, end, 1);

    for (auto&& x : vec) {
        std::cout << x << '\n';
    }
}

Context

StackExchange Code Review Q#156160, answer score: 5

Revisions (0)

No revisions yet.