mohitanand001 / underscore_cpp

underscore_cpp
MIT License
7 stars 30 forks source link

Change group_by so it the key can take any generic value. #59

Closed mohitanand001 closed 5 years ago

mohitanand001 commented 5 years ago

https://github.com/farziengineer/underscore_cpp/blob/2fd26972c8042ae5ed29304e8b072e78807e7df3/underscore.cpp#L313 As of now, it only takes an int for the key.

gotham13 commented 5 years ago

group by return type of the grouping function can be good

mohitanand001 commented 5 years ago

@Gotham13121997 exactly.

gubatron commented 5 years ago

think something along the lines of this would fly???

template<typename Container, typename Function, typename FunctionReturnType>
    std::map<FunctionReturnType, std::vector<typename Container::value_type> >  group_by(Container &container, Function function) 
    {
std::map<FunctionReturnType, std::vector<typename Container::value_type> > result;

so then you can go:

std::map<bool, std::vector<int>> grouped_results = _::group_by(some_int_container, some_boolean_returning_function_that_takes_an_int_argument);
gubatron commented 5 years ago

when this one is done, then it makes sense to work on #61

mohitanand001 commented 5 years ago

@gubatron want to give it a try, go ahead ☺️

gubatron commented 5 years ago

alright, will submit a PR in a couple hours if everything goes smoothly with building and my dev environment.

rigby-dane commented 5 years ago

I can also take this, if @gubatron doesn't have time for it.

mohitanand001 commented 5 years ago

Hi @Rigbert, I think @gubatron might need more time. @gubatron please keep us updated.

gubatron commented 5 years ago

I got something working already, but I'm not happy with it, iterating over it some more.

It looks like this now:

Usage:

// takes a value as the way for the compiler to be hinted on what the key type is
std::map<bool, std::vector<int> > mp = _::group_by(vec, is_odd, false);
...
std::map<bool, std::vector<int> > mp = _::group_by(vec, is_odd, true);
...
std::map<int, std::vector<int> > mp = _::group_by(vec, is_odd, 1337);

The signature looks like this:

template <typename Container, typename Function, typename KeyType>
auto group_by(Container &container, Function function, KeyType keyHint) -> std::map< KeyType, std::vector<typename Container::value_type> >;

I was working with auto on the left hand side because I was playing with decltype as a way to get what the KeyType would be.

My goal is to have it work like this, without passing values as type hints to the compiler

std::map<int, std::vector<int> > mp = _::group_by<int>(vec, is_odd);
gubatron commented 5 years ago

It also works like this, without auto:

template <typename Container, typename Function, typename KeyType>
std::map< KeyType, std::vector<typename Container::value_type> > group_by(Container &container, Function function, KeyType keyHint);

but again, not happy until I get the usage to allow me to specify the type itself, but dealing with lots of errors

gubatron commented 5 years ago

I've left a question on stackoverflow for now, just tried for another hour with no success

mohitanand001 commented 5 years ago

Great, will have to see what can be done. We can wait one more day to see if someone answers on stack overflow

gubatron commented 5 years ago

damn, already a couple of answers

mohitanand001 commented 5 years ago

Damn 😀

gubatron commented 5 years ago

got it working like this: std::map<bool, std::vector<int> > mp = _::group_by<bool>(vec, is_odd);

but now I'm gonna make it even more magic so it works like this: std::map<bool, std::vector<int> > mp = _::group_by(vec, is_odd);

using decltype and auto, my issue was the order of the template parameters, good lesson learned.

gubatron commented 5 years ago

There you go guys/gals, sorry for the delay https://github.com/farziengineer/underscore_cpp/pull/64

gotham13 commented 5 years ago

closed via #64

mohitanand001 commented 5 years ago

Thanks a lot @gubatron. Learnt a lot here.