18

I often find myself using std::sort, std::max_element, and the like with a lambda that simply invokes a member function

std::vector<MyType> vec;
// populate...
auto m = std::max_element(std::begin(vec), std::end(vec),
    [](const MyType& a, const MyType& b) { return a.val() < b.val()})

this feels like a waste of characters and a loss of clarity. I'm aware that I could write another function/callable and pass a function pointer/callable object to these algorithm functions, but I often need to do this sort-by just once in a program and it doesn't strike me as a good way of addressing the problem. What I want to do, ideally is say:

auto m = std::max_element(std::begin(vec), std::end(vec), &MyType::val);

and have the objects be sorted by their val()s. Is there some part of the stdlib I'm overlooking that could assist me with this? or another simple way of doing it? I'd like to make what this is sorting or searching by as obvious as possible.

I'm aware that just &MyType::val isn't enough, I am looking for something that could perhaps wrap it, or provide a similar functionality without obscurring the meaning.

Ryan Haining
  • 35,360
  • 15
  • 114
  • 174

5 Answers5

16

You can use std::mem_fn (or std::tr1::mem_fn)

int main()
{
    std::vector<MyType> vec;

    auto m = std::max_element(std::begin(vec), std::end(vec), compare_by(std::mem_fn(&MyType::field)));
}

Of course, this assumes that you have a utility like compare_by in your toolbox (as you should :)):

template <typename F>
struct CompareBy {
    explicit CompareBy(F&& f) : f(std::forward<F>(f)) {}
    template <typename U, typename V> 
        bool  operator()(U const& u, V const& v) const {
            return f(u) < f(v);
        }

private:
    F f;
};

template <typename F>
CompareBy<F> compare_by(F&& f) { return CompareBy<F>(std::forward<F>(f)); }

See it Live On Coliru

sehe
  • 374,641
  • 47
  • 450
  • 633
  • 3
    ok, that is a valuable alternative, but (@Ryan) please tell me why this should be better than a single-line lambda? – davidhigh May 11 '14 at 20:11
  • 1
    It was meant like this (and I already corrected it before reading your suggestion)... I ask it here as I guess he was among the upvoters. – davidhigh May 11 '14 at 20:15
  • @davidhigh it's less redundant, it's more generic (can accept any functor, the sample also accepting mixed type parameters, leveraging potential implicit conversions). For me the leading reason would be to avoid errors (the expression `f(a) < f(b)` repeats both the `f()` application and the conceptual `<`) – sehe May 11 '14 at 20:38
  • `template> auto less_by( F&& f_in ) { return [f = std::forward(f_in)](auto&& left, auto&& right)->bool { return Comp()(f(std::forward(left)), f(std::forward(right))); }; }` -- C++1y style. – Yakk - Adam Nevraumont May 11 '14 at 20:53
  • @Yakk you know... as much as I _love_ poly-lambdas, I really prefer the hand written functor here. This is just outlandish. Perhaps you can cut the complexity by **not** going meta with the configurable `Comp` :) Also, I'd suggest that having a predicate that _moves_ it's arguments is _very undeserirable_ at best, I'd keep it at `template auto less_by(F&& f_in) { return [f = std::forward(f_in)](auto const& a, auto const& b) { return f(a) < f(b); };` – sehe May 11 '14 at 21:12
  • So much code with something that can be done with `std::less` ! Use it with `std::bind` to get the same result without the need for additional components (see my answer) – Nikos Athanasiou May 11 '14 at 22:08
  • @sehe it only moves rvalues ;) By default, perfect forward when forwarding. The goal was library quality while being ridiculously terse. Sadly did not find justification for triple lambda... – Yakk - Adam Nevraumont May 11 '14 at 23:15
  • @Yakk You and I have different goals :) About moving, my point is exactly this: I don't think one can ever usefully _move_, even from _rvalue-ref_ in algorithms that use a comparator. Think about it. (So, perfect forwarding is useless at best, and dangerous if not). On second thought I admit that having `less<>` parameterized _can_ be useful (e.g. replace it with `greater<>` :)). – sehe May 12 '14 at 06:58
  • @NikosAthanasiou Duh. It can be done without std::less too. The point is, iff you use a helper like this, you can simplify all your call sites. using `std::bind`+`std::less` just _complicates_ the call site, for both the reader and the compiler. I think the lambda then is *much* clearer. – sehe May 12 '14 at 07:11
  • 1
    @sehe Ofcourse, I'm just staying in context and consider non using lambdas a precondition. I'd also use one unless I had std::algorithms that alternate among using operations like `less` / `greater` / `greater_equal` etc, and `std::` made my life easier – Nikos Athanasiou May 12 '14 at 08:27
  • @sehe easy: `std::vector` -- returns rvalues from which moving is non destructive. A more complex pseudo-reference could have expensive to copy pseudo references: a `move` could add efficiency, and as perfect forwarding is a thing pseudo references cannot move their referant on `move`. – Yakk - Adam Nevraumont May 12 '14 at 13:05
  • @Yakk. You missed my point. I didn't say "cannot safely be moved from". I said "cannot _usefully_ be moved from". _(It's quite simple, in my view. Moving from operand in a STL comparator is just not useful. If it's safe, then passing by reference is likely more efficient, or equally efficient at best.)_ – sehe May 12 '14 at 13:07
  • 3
    Forwarding here is nonsensical. A move doesn't add any efficiency over *doing nothing* which is what you get when you just pass by reference. There are no copies in the pre-C++11 version of this. Why would you want to add moves to it? – R. Martinho Fernandes May 12 '14 at 13:14
  • @R.MartinhoFernandes `operator<` (or `Comp`) could be taking by value. The `move`/`forward` does **nothing** unless it does. If it does take by value, the `move`/`forward` could be useful, **if** the iterators return pseudo-reference rvalues on dereference, **and** those pseudo-references are cheaper to `move` than to copy (for example, they contain `std::shared_ptr`, where `copy` requires an atomic operation). Now, I did not have this in mind when I wrote the perfect forwarding code, I simply perfect forward by default when I'm moving parameters through my code into code I do not control. – Yakk - Adam Nevraumont May 12 '14 at 13:35
  • 1
    _It's probably a "good" habit to perfectly-forward when possible in library code. But it's probably an even "better" habit to think about what you're doing when possible._ – sehe May 12 '14 at 13:36
3

You can do it without introducing any new functions (templated or not).

Just use bind and std::less

auto m = std::max_element(vec.begin(), vec.end(), 
    bind(less<>(), bind(&MyType::val, _1), bind(&MyType::val, _2)));
Nikos Athanasiou
  • 29,616
  • 15
  • 87
  • 153
  • Where `less<>` is a C++1y feature. – dyp May 11 '14 at 22:01
  • @dyp Works in VS2013, make it `less` and it'll work in g++4.8 – Nikos Athanasiou May 11 '14 at 22:05
  • 1
    Yes, that's one of the (few) features MSVC supports "ahead of time", like `make_unique`. IIRC both have been proposed my Microsoft employees. – dyp May 11 '14 at 22:07
  • 1
    Good point. I love `std::bind`. However, in this case, the lambda is clearly superior (this still repeats the same parts). Also, nested binds for functional composition can be unwieldy and surprising (http://stackoverflow.com/q/18519087/85371, [boost::apply](http://www.boost.org/doc/libs/1_55_0/libs/bind/bind.html))). +1 for informative, though. PS. No need for the `val()` member, I think [`std::bind` handles p-t-m just fine](http://coliru.stacked-crooked.com/a/563132a9413866a6) – sehe May 12 '14 at 07:17
1

A templated comparator could help you:

template <typename StructureType,
          typename MemberType,
          MemberType StructureType::*member>
bool comparator(const StructureType& the_first, const StructureType& the_second)
{
  return the_first.*member < the_second.*member;
}

http://ideone.com/K8ytav

A bit of type traits magic could certainly allows you to avoid writing the type.

Johan
  • 3,728
  • 16
  • 25
0

How about once overloading operator< for your custom type? This can be naturally done inside the class (or directly next to it), and then no further argument is required beside the iterators.

Passing your val() function isn't possible, as you must pass a binary operator.

EDIT: Having read the other valuable alternatives (also sehe's nice response), I want to confirm what I already mentioned in the comment below: In my opinion, nothing beats the readibility, locality and also flexibility of a lambda expression (--on the risk of writing some passages twice).

@Ryan Haining: I suggest you to keep it as in your original post.

davidhigh
  • 14,652
  • 2
  • 44
  • 75
  • because it often doesn't make sense to say that something is less than another. if I want to sort employees by salary, that would mean overloading `operator<` to compare by salaries. this is not expressive because it's not intuitive to say one person is "less than" another, and because if I later need them sorted by age, I'm back to square one. – Ryan Haining May 11 '14 at 19:26
  • Then I guess the lambda is the shortest possibility you can expect. Of course you could also make somehow stack together `std::bind` with `std::less`, but this will be harder to read, imo. – davidhigh May 11 '14 at 19:27
0

An improvement on sehes answer, so as to avoid needing to use std::mem_fn would be provide a pointer to member overload for the compare_by function.

example usage

std::sort(std::begin(vec), std::end(vec), compare_by(&MyType::field));
std::sort(std::begin(vec), std::end(vec), compare_by(&MyType::field, std::greater<>{}));

the code to implement

#include <functional> // std::less
#include <utility> // std::move
#include <type_traits> // std::is_invocable_r
// Forward declaration
template<typename R, typename T, typename F = std::less<R>>
auto compare_by(R T::*, F = F{});

// Implementation
namespace detail {
template<typename T, typename F>
struct compare_by_t;

template<typename R, typename T, typename F>
struct compare_by_t<R T::*, F> : private F
{
    compare_by_t(F&& f, R T::*m): F{std::move(f)}, _member{m} {}
    R T::* _member;
    bool operator()(T const& x, T const& y) const
    {
        return F::operator()(x .* _member, y .* _member);
    }
};
} // detail

template<typename R, typename T, typename F>
auto compare_by(R T::* member, F f)
{
    static_assert(std::is_invocable_r<bool, F, R, R>::value);
    return detail::compare_by_t<R T::*, F>{ std::move(f), member };
}
taiyu
  • 3
  • 4