CODEWITHSUNDEEP
arraystemplatesc++11vectorstd
PekkaRo
PekkaRo

Reputation: 80

Comparison function for std::vector of std::arrays

I want to handle a large number of arrays of doubles. All arrays are of the same length. I decided to use std::vector of std::arrays. For data scaling I need to find minima and maxima of all the values. How I figured I could do this is as follows:

#include <algorithm>
#include <vector>
#include <array>

class ChannelData {
...
template<std::size_t N>
bool static compareX(std::array<double, m_nelem> const& a, std::array<double, m_nelem> const& b) {
    return a[N] < b[N];
}

std::vector<std::array<double, m_nelem> > m_data;
std::array<<std::array<double, 2> m_nelem> m_minmax;

void find_minmax_x(){
    for (int i = 0; i < m_nelem; i++){
        auto minmax = std::minmax_element(m_data.begin(), m_data.end(), compareX<i>);
        m_minmax[i][0] = (*minmax.first)[i];
        m_minmax[i][1] = (*minmax.second)[i];
    }
}

This does not compile. That is because the nontype parameter i that I use to to instantiate compareX<N> in function find_minmax_x is not a constant expression.

To my understanding, the compiler does not know which versions to instantiate at compile time, even though in this case it is fairly obvious. What I ended up doing was manually unrolling the for-loop, but that is ugly. Could you recommend a more aesthetic way to accomplish this?

Upvotes: 2

Views: 28

Answers (1)

Yakk - Adam Nevraumont
Yakk - Adam Nevraumont

Reputation: 275500

struct compareX {
  std::size_t N;
  bool operator()(std::array<double, m_nelem> const& a, std::array<double, m_nelem> const& b) const {
    return a[N] < b[N];
  }
};

...

void find_minmax_x(){
  for (int i = 0; i < m_nelem; i++){
    auto minmax = std::minmax_element(m_data.begin(), m_data.end(), compareX{i});
    m_minmax[i][0] = (*minmax.first)[i];
    m_minmax[i][1] = (*minmax.second)[i];
  }
}

if your value isn't a compile-time value, don't pass it as a template non-type parameter. Store it.

std algorithms will consume invokable objects as easily as they'll consume function pointers.

Writing compareX out of line is a bit annoying. We can move it inline:

void find_minmax_x(){
  for (int i = 0; i < m_nelem; i++){
    auto compareX = [i](std::array<double, m_nelem> const& a, std::array<double, m_nelem> const& b) {
      return a[i] < b[i];
    };
    auto minmax = std::minmax_element(m_data.begin(), m_data.end(), compareX);
    m_minmax[i][0] = (*minmax.first)[i];
    m_minmax[i][1] = (*minmax.second)[i];
  }
}

which is a bit verbose right there, because the types in question are huge. In C++14 we do away with those huge types and get:

void find_minmax_x(){
  for (int i = 0; i < m_nelem; i++){
    auto compareX = [&](auto&& a, auto&& b) {
      return a[i] < b[i];
    };
    auto minmax = std::minmax_element(m_data.begin(), m_data.end(), compareX);
    m_minmax[i][0] = (*minmax.first)[i];
    m_minmax[i][1] = (*minmax.second)[i];
  }
}

which automatically creates an invokable object with a template operator() that captures i by reference.

I typically have a projection order_by comparison:

template<class F, class O=std::less<>>
auto order_by( F&& f, O&& o = {} ) {
  return [f = std::forward<F>(f), o = std::forward<O>(o)](auto&& lhs, auto&& rhs)->bool{
    return o(f(lhs), f(rhs));
  };
};

(C++14 again, can be written in C++11) which would reduce your code to:

void find_minmax_x(){
  for (int i = 0; i < m_nelem; i++){
    auto minmax = std::minmax_element(
      m_data.begin(), m_data.end(), 
      order_by([i](auto&&a){return a[i];})
    );
    m_minmax[i][0] = (*minmax.first)[i];
    m_minmax[i][1] = (*minmax.second)[i];
  }
}

Upvotes: 4

Related Questions