11

The standard algorithms min and max can be compared against a single value. However, the minmax algorithm return value cannot be compared against a pair of values: 

#include <algorithm>
#include <utility>

template<class T1, class T2>
constexpr auto make_cref_pair(T1&& t1, T2&& t2)
{
    return std::pair<T1 const&, T2 const&>(std::forward<T1>(t1), std::forward<T2>(t2));
}

int main()
{
    static_assert(std::min(2, 1) == 1); // OK
    static_assert(std::max(2, 1) == 2); // OK
    //static_assert(std::minmax(2, 1) == std::make_pair(1, 2)); // ERROR, const int& vs int pair comparison
    static_assert(std::minmax(2, 1) == std::pair<const int&, const int&>(1, 2)); // OK
    static_assert(std::minmax(2, 1) == make_cref_pair(1, 2)); // OK
}

Live Example

The reason is that make_pair(2, 1) returns a pair<int, int> and minmax(1, 2) returns a pair<const int&, const int&>. There are no referenceness-mixing operator== overloads for pair.

The fix is then to explicitly write std::pair<const int&, const int&>(int, int) or to wrap this in a home-made make_cref_pair function.

Questions: is there a cleaner way to compare the minmax return value against a pair of values? And did I correctly handle the references in my make_cref_pair?

Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
TemplateRex
  • 69,038
  • 19
  • 164
  • 304
  • 1
    I fear that `make_cref_pair` may not be legal (life time extension doesn't apply to sub-objects, AFAIK) – sehe May 20 '16 at 19:59
  • @sehe yikes. Does the same apply to the naked `pair(1,2)`? – TemplateRex May 20 '16 at 20:00
  • It would be the same. I'm not really sure about this, but it does seem like playing on the edge there – sehe May 20 '16 at 20:01
  • Maybe another way would be to provide the appropriate `operator==` overload by oneself. It's probably safer than `cref_pair` when it comes to object life, but on the other side it's introducing an "unexpected" overload. – luk32 May 20 '16 at 20:03
  • @sehe It seems no more dangerous than, say, `forward_as_tuple`. – T.C. May 20 '16 at 21:33
  • @T.C. that would forward temps as rvalues, making them values, not refs, in the tuple? – sehe May 21 '16 at 00:23
  • @sehe it returns a tuple of references. – T.C. May 21 '16 at 00:43

3 Answers3

12

std::minmax has an initializer_list overload. This returns a non-const non-reference pair:

static_assert(std::minmax({2, 1}) == std::make_pair(1, 2));

Unfortunately this may be less performant, since the complexities respectively are "exactly one comparison" and "at most (3/2) * t.size() applications of the corresponding predicate".

user6362820
  • 136
  • 1
  • 5
  • well, I will only use the comparison at compile-time (it's for initializing a variable template, hence the C++14 tag, I need the `constexpr`). Thanks, this is a nice answer! – TemplateRex May 20 '16 at 20:15
  • do you perhaps also know the answer to the lifetime issues using a `pair`? – TemplateRex May 20 '16 at 20:25
  • 2
    @TemplateRex The issue with dangling references is only a problem if you try to access `first` or `second` afterwards. AFAIK this is a problem with `minmax` as well. A plausible workaround is to use `reference_wrapper`. – user6362820 May 20 '16 at 20:41
5

One thing you could do is take advantage of the std::minmax overload that takes a std::initializer_list<T> and returns a std::pair<T,T>. Using that you could have

int main()
{
    const int a = 10, b = 20;
    static_assert(std::minmax({2, 1}) == std::make_pair(1, 2));
    static_assert(std::minmax({a, b}) == std::make_pair(a, b));
}

Which will compile and allows you to get rid of make_cref_pair. It does call std::minmax_element so I am not sure if this decreases the efficiency or not.

NathanOliver
  • 171,901
  • 28
  • 288
  • 402
3

One option is to explicitly convert the left-hand side to std::pair<int,int>:

#include <algorithm>
#include <utility>

template <typename T1, typename T2>
constexpr std::pair<T1,T2> myminmax(const T1& t1, const T2& t2)
{
    return std::minmax(t1,t2);
}

int main()
{
    static_assert(myminmax(2, 1) == std::make_pair(1, 2));
}
mindriot
  • 5,413
  • 1
  • 25
  • 34