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

"Zip-like" functionality with C++11's range-based for-loop

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

Problem

My goal was to get the following code to work:

#include
#include
#include
#include
#include
#include"functional.hpp"

int main( )
{
    std::vector a{1.0, 2.0, 3.0, 4.0};
    std::list b;
    b.push_back('a');
    b.push_back('b');
    b.push_back('c');
    b.push_back('d');
    std::array c{5,4,3,2,1};

    auto d = zip(a, b, c);

    for (auto i : zip(a, b, c) )
    {
        std::cout (i) (i) (i) (i) (i) (i) (i) = 5.0;
        //std::cout (i) (i) (i) << std::endl;
        //std::cout << i1 << ", " << i2 << ", " << i3 << std::endl;
    }
}


With the output:

1, a, 5
2, b, 4
3, c, 3
4, d, 2
5, a, 5
5, b, 4
5, c, 3
5, d, 2


The source for "functional.hpp" is:

```
#pragma once
#include
#include
#include

/***
// helper for tuple_subset and tuple_tail (from http://stackoverflow.com/questions/8569567/get-part-of-stdtuple)
***/
template
struct ct_integers_list {
template
struct push_back
{
typedef ct_integers_list type;
};
};

template
struct ct_iota_1
{
typedef typename ct_iota_1::type::template push_back::type type;
};

template <>
struct ct_iota_1
{
typedef ct_integers_list<> type;
};

/***
// return a subset of a tuple
***/
template
auto tuple_subset(const Tuple& tpl, ct_integers_list)
-> decltype(std::make_tuple(std::get(tpl)...))
{
return std::make_tuple(std::get(tpl)...);
// this means:
// make_tuple(get(tpl), get(tpl), ...)
}

/***
// return the tail of a tuple
***/
template
inline std::tuple tuple_tail(const std::tuple& tpl)
{
return tuple_subset(tpl, typename ct_iota_1::type());
// this means:
// tuple_subset(tpl, ..)
}

/***
// increment every element in a tuple (that is referenced)
***/
template
inline typename std::enable_if::type
increment(std::tuple& t)
{ }

template
inline typenam

Solution

I have not much to say. Your code reads quite good, which is rather pleasant. Here are a few tidbits though:

typedef

If you are willing to write modern code, you should consider dropping typedef and using using everywhere instead. It helps to be consistent between regular alias and alias template. Moreover, the = symbol help to visually split the new name and the type it refers to. And the syntax is also consistent towards the way you can declare variables:

auto i = 1;
using some_type = int;


Perfect forwarding

It's clear that you already used it. But there are some other places where it would make sense to use it:

template 
auto tuple_subset(Tuple&& tpl, ct_integers_list)
    -> decltype(std::make_tuple(std::get(std::forward(tpl))...))
{
    return std::make_tuple(std::get(std::forward(tpl))...);
    // this means:
    //   make_tuple(get(tpl), get(tpl), ...)
}


std::enable_if

While using std::enable_if in the return type of the functions, I find that it tends to make it unreadable. Therefore, you may probably want to move it to the template parameters list instead. Consider your code:

template
inline typename std::enable_if::type
increment(std::tuple& t)
{
    std::get(t)++ ;
    increment(t);
}


And compare it to this one:

template::type>
inline void increment(std::tuple& t)
{
    std::get(t)++ ;
    increment(t);
}


Pre-increment vs. post-increment

Depending on the type, ++var may be faster than var++. It does not change anything for int but if your container contains large type, remember that the ++ in var++ is generally defined as:

auto operator++(int)
    -> T&
{
    auto res = var;
    ++var;
    return res;
}


As you can see, yet another copy of the incremented variable is made and ++var is called. Therefore, you may want to use ++var instead of var++ in a generic context.

Miscellaneous tidbits

template
zipper& operator=(zipper& rhs) { ... }


You may want to pass a const zipper& instead of a zipper&.

zipper::iterator& begin() {
    return begin_;
}

zipper::iterator& end() {
    return end_;
}


You may also want to provide the functions begin() const, end() const, cbegin() const and cend() const if you want this set of functions to be complete and consistent towards the STL. Also, some operator== would be great for zipper::iterator.

Also, I like the new function syntax which IMHO helps to separate the function return type and its name, which is especially useful when the said return type is long. However, I read that some people don't like it, so it's up to your own preferences.

Conclusion

Generally speaking, your code was good and it works, which is even better. I've provided a few tips, but there are probably many other things that you could do to improve it. It will always be up to you when it concerns the readability though, preferences matter in that domain :)

Edit: ok, so apparently yout example worked fine, but @Barry's answer seem to highlight more serious problems. You might want to accept it instead.

Code Snippets

auto i = 1;
using some_type = int;
template <size_t... indices, typename Tuple>
auto tuple_subset(Tuple&& tpl, ct_integers_list<indices...>)
    -> decltype(std::make_tuple(std::get<indices>(std::forward<Tuple>(tpl))...))
{
    return std::make_tuple(std::get<indices>(std::forward<Tuple>(tpl))...);
    // this means:
    //   make_tuple(get<indices[0]>(tpl), get<indices[1]>(tpl), ...)
}
template<std::size_t I = 0, typename... Tp>
inline typename std::enable_if<(I < sizeof...(Tp)), void>::type
increment(std::tuple<Tp...>& t)
{
    std::get<I>(t)++ ;
    increment<I + 1, Tp...>(t);
}
template<std::size_t I = 0, typename... Tp,
        typename = typename std::enable_if<I < sizeof...(Tp), void>::type>
inline void increment(std::tuple<Tp...>& t)
{
    std::get<I>(t)++ ;
    increment<I + 1, Tp...>(t);
}
auto operator++(int)
    -> T&
{
    auto res = var;
    ++var;
    return res;
}

Context

StackExchange Code Review Q#30846, answer score: 15

Revisions (0)

No revisions yet.