CODEWITHSUNDEEP
c++c++11
Tim
Tim

Reputation: 413

Can I use std::minmax_element in a member initializer list?

This is a class that contains a const std::vector and stores its min and max:

#include <vector>
#include <algorithm>

class MyClass {

public:
    const std::vector<int> v;
    const std::pair<int, int> minmax_v;

    MyClass(const std::vector<int> & v_init)
    : v(v_init),
    minmax_v(*(std::min_element(v.begin(), v.end())), *(std::max_element(v.begin(), v.end()))) {}

};

It is possible to somehow use std::minmax_element to initialize minmax_v? The idea is is to improve performance.

Of course, one could use std::minmax_element in the constructor body and drop the const for minmax_v but that doesn’t feel right either.

Upvotes: 4

Views: 275

Answers (2)

Jens
Jens

Reputation: 9406

Since you are asking this to improve performance, I want to point out that it may be faster to use a custom loop in this case. In your example, there are two operations which both iterate over the vector: the initialization of v and std::minmax_element both traverse the input vector. Iterating twice seems not optimal, so it may be better to merge the operations into one loop.

class MyClass {
private:
    MyClass(std::tuple<int, int, std::vector<int>>&& x): v( std::get<2>(x) ), minmax_v( std::get<0>(x), std::get<1>(x)) {

}

    static std::tuple<int, int, std::vector<int>> init(std::vector<int> const& v_init) {
        std::vector<int> v;
        v.reserve(v_init.size());
        int min = v_init.front();
        int max = v_init.front();
        for(auto const& x: v_init) {
            v.push_back(x);

            if (min > x) {
                min = x;
            }

            if (max < x) {
                max = x;
            }
        }

        return std::make_tuple(min,max, std::move(v));
    }

public:
    const std::vector<int> v;
    const std::pair<int, int> minmax_v;

    MyClass(const std::vector<int> & v_init): MyClass(init(v_init)) {}
};

I am assuming that the input vector is not empty as the original example also dereferences the result std::min_element and std::max_element without checking it against v_init.end(). Since const members cannot be initialized in the constructor's body I had to come up with a work-around using a helper tuple.

I also want to say that I would probably prefer using STL algorithms instead of the hand-written loop until I can see a performance issue. Modern compilers probably merge the two loop into a single loop and produce efficient code.

Upvotes: 1

Jarod42
Jarod42

Reputation: 217235

Use a delegating constructor to obtain the pair of minmax_element() results in a single argument:

class MyClass {
public:
    const std::vector<int> v;
    const std::pair<int, int> minmax_v;

    MyClass(const std::vector<int> & v_init)
    : MyClass(v_init, std::minmax_element(v_init.begin(), v_init.end()))
    {}

private:
    MyClass(const std::vector<int> & v_init,
            std::pair<std::vector<int>::const_iterator,
                      std::vector<int>::const_iterator> p)
        : v(v_init), minmax_v(*p.first, *p.second)
    {}

};

Upvotes: 7

Related Questions