Question

I was playing around with implementing group_by method in a generic way and I have maybe implemented it(except it doesnt work for C arrays), but still code looks ugly to me...

Is there easier way to do what I want(+ for it to work for all containers and also C arrays(I dont know how to make it work for C arrays (T T) ) ?
If it is not obvious Im talking about types for std::multimap... BTW I know C++14 will remove need to type this 2 times( auto will know what is type that now we write after -> )

// group_by.cpp : Defines the entry point for the console application.
//
#include <iostream>
#include <map>
#include <deque>
#include <cstdint>
#include <functional>
#include <algorithm>
#include <iostream>
template<typename Cont, typename F >
auto group_by (Cont c, F f) -> std::multimap< typename std::remove_reference<decltype(*std::begin(c))>::type, decltype(f(*std::begin(c)))> 
{
    std::multimap<typename std::remove_reference<decltype(*std::begin(c))>::type , decltype(f(*std::begin(c)))>  result;
    std::for_each(std::begin(c), std::end(c), 
        [&result,&f](typename Cont::value_type elem)
    {
        auto key = f(elem);
        result.insert(std::make_pair(key,elem));
    }
    );
    return result;
}
int main()
{
    std::deque<uint64_t> dq;
    std::deque<uint64_t>::value_type asghuitl;
    dq.push_back(1);
    dq.push_back(2);
    dq.push_back(11);
    dq.push_back(21);
    auto result = group_by(dq, [] (uint64_t x){return x%10;});

}
Was it helpful?

Solution

First step: make it work with C-style arrays.

Second step: Use range-based for loop. Much less problems / effort.

Third step: Make it a bit nicer using a template alias.

#include <iostream>
#include <map>
#include <deque>
#include <cstdint>
#include <functional>
#include <algorithm>
#include <iterator>

template < typename Elem, typename Res >
using return_type = std::multimap < typename std::remove_reference<Elem>::type,
                                    Res>;

template<typename Cont, typename F >
auto group_by (Cont const& c, F const& f)
    -> return_type < decltype(*std::begin(c)), decltype(f(*std::begin(c))) >
{
    return_type< decltype(*std::begin(c)),
                 decltype(f(*std::begin(c))) >  result;

    for(auto const& e : c)
    {
        auto const& key = f(e);
        result.insert( std::make_pair(key,e) );
        // alternatively, just:
        // result.emplace( f(e), e );
    }

    return result;
}

int main()
{
    char const foo[] = "hello world";
    auto result = group_by(foo, [] (uint64_t x){return x%10;});
}

Visual-Studio-supported version:

template < typename Cont, typename F >
auto group_by (Cont const& c, F const& f)
  -> std::multimap
     <
       typename std::remove_reference<decltype(*std::begin(c))>::type,
       decltype(f(*std::begin(c)))
     >
{
    using key_ref = decltype( *std::begin(c) );
    using key_type = typename std::remove_reference < key_ref > :: type;
    using value_type = decltype( f(*std::begin(c)) );

    std::multimap < key_type, value_type > result;

    for(auto const& e : c)
    {
        result.emplace( f(e), e );
    }

    return result;
}

Fourth step: Use iterators instead of passing the container.

#include <iostream>
#include <map>
#include <deque>
#include <cstdint>
#include <functional>
#include <algorithm>
#include <iterator>

template < typename Elem, typename Res >
using return_type = std::multimap< typename std::remove_reference<Elem>::type,
                                   Res >;

template < typename FwdIt, typename F >
auto group_by (FwdIt beg, FwdIt end, F const& f)
    -> return_type < decltype(*beg), decltype(f(*beg)) >
{
    return_type < decltype(*beg), decltype(f(*beg)) >  result;

    for(FwdIt i = beg; i != end; ++i)
    {
        result.emplace( f(*i), *i );
    }

    return result;
}

int main()
{
    char const foo[] = "hello world";
    auto result = group_by( std::begin(foo), std::end(foo),
                            [] (uint64_t x){return x%10;}   );
}

Visual-Studio-supported version:

template < typename FwdIt, typename F >
auto group_by (FwdIt beg, FwdIt end, F const& f)
  -> std::multimap
     <
       typename std::remove_reference<decltype(*std::begin(c))>::type,
       decltype(f(*std::begin(c)))
     >
{
    using key_ref = decltype( *std::begin(c) );
    using key_type = typename std::remove_reference < key_ref > :: type;
    using value_type = decltype( f(*std::begin(c)) );

    std::multimap < key_type, value_type > result;

    for(FwdIt i = beg; i != end; ++i)
    {
        result.emplace( f(*i), *i );
    }

    return result;
}

OTHER TIPS

The code you have will work for arrays with very little change. Firstly, you need to #include <iterator>, for std::begin and std::end. Secondly, you should be passing by const&. Finally, a few typedefs will help make the rest of the function a bit more readable. It ends up looking like:

template<typename Cont, typename F >
auto group_by (const Cont& c, F f) -> 
  std::multimap< typename std::remove_reference<decltype(*std::begin(c))>::type,    
                 decltype(f(*std::begin(c)))> 
{

    typedef typename std::remove_reference<decltype(*std::begin(c))>::type value_type;
    typedef decltype(f(*std::begin(c))) result_type;

    std::multimap<value_type, result_type>  result;
    std::for_each(std::begin(c), std::end(c), 
        [&result,&f](value_type elem)
    {
        auto key = f(elem);
        result.insert(std::make_pair(key,elem));
    }
    );
    return result;
}

Can you make this less ugly? Well, probably not much. You could utilize traits to get the value type of what's being passed in (similar to iterator_traits):

template <typename T>
struct container_traits
{
    typedef typename T::value_type value_type;
};

template <typename T, std::size_t N>
struct container_traits<T[N]>
{
    typedef T value_type;
};

template <typename T>
struct container_traits<T*>
{
    typedef T value_type;
};

Utilizing this and std::result_of (which requires type_traits):

template<typename Cont, typename F>
auto group_by (const Cont& c, F f) -> 
    std::multimap<
      typename container_traits<Cont>::value_type, 
      typename std::result_of<F(typename container_traits<Cont>::value_type)>::type>
{
    typedef typename container_traits<Cont>::value_type value_type;
    typedef typename 
      std::result_of<F(typename container_traits<Cont>::value_type)>::type result_type;

    std::multimap<value_type, result_type>  result;
    std::for_each(std::begin(c), std::end(c), 
        [&result,&f](value_type elem)
    {
        auto key = f(elem);
        result.insert(std::make_pair(key,elem));
    }
    );
    return result;
}

However, this requires more code. The naming might be slightly clearer as to what exactly is going on, but the decltype solution is probably "cleaner".

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top