Simplifying minimum search of two dimensional vector with complex struct

Question

Is there a way to simplify this function? I tried to use the std::min_element function inside the lambda, but i don't get it to work with *var.

const auto miniFunc = [](const std::vector<std::vector<KEntryContact>> & entryContact, double KEntryContact::*var) -> double
{
    double min = DBL_MAX;
    for (const std::vector<KEntryContact> &entryvec : entryContact) {
        for (const KEntryContact &entry : entryvec) {
            min = std::min(min, entry.*var);
        }
    }

    return min;
};

The function can then be used like this:

double rAMin = miniFunc(tPOC->getEntryContact(), &KEntryContact::r1);

Answer 1

Simplifying isn't enough. I mean, your function is already quite simple, and complexity isn't what's questionable about it. To improve it, I'd suggest to:

handle empty input better

The way you handle empty input is questionable.

First, if you want to initialize min with the highest value for a double, use std::numeric_limits<double>::max() in <limits> instead of a macro.

Second, what if entryContact is empty, or if all its members are empty? You'll return DBL_MAX. And what if the smallest element inside is equal to DBL_MAX? You'll return DBL_MAX. Your return value is ambiguous.

At that point, you need to decide if an empty input is meaningless or meaningful. If it's meaningless, you might handle it by throwing an exception; if not, your return type should represent the fact that there are no smallest element if your input vector is empty. std::optional is a good candidate if c++17 is available to you.

Use the standard library more

Beyond std::optional, I would recommend to use algorithms from the standard library over raw loops. std::min_element will fit the bill for the inner vectors, but not for the outer one. You could use std::accumulate (std::reduce would be better but is only available in clang as for now).

Improve your code readability

I would advocate that using the keyword auto makes for a more readable code sometimes, and gives more flexibility, especially for lambdas because they are defined locally.

So here's how I would write it:

#include <algorithm>
#include <numeric>
#include <optional>
#include <limits>

const auto miniFunc = [](const auto& entry_vectors, auto member_var) {
    auto value  = [=] (auto&& entry)            { return entry.*member_var; };
    auto cmp    = [=] (auto&& lhs, auto&& rhs)  { return value(lhs) < value(rhs); };
    auto min    = [=] (auto&& init, auto&& vec)   {
        auto vec_min = std::min_element(vec.begin(), vec.end(), cmp);
        if (vec_min == vec.end()) return init;
        auto candidate = std::make_optional(value(*vec_min));
        return init ? std::min(init, candidate) : candidate;
    };
    return std::reduce(entry_vectors.begin(), entry_vectors.end(), std::optional<double>{}, min);
};

Edit: cf @TobySpeight's comment

Answer 2

What you are wishing for is called a projector, and it is a common need so Eric’s Range.v3 library features optional projector arguments on most of the range algorithms.

To get std::min to look at a single data member of the arguments, just supply your own comparison function. ‘What I mean by ordering KEntryContact objects is to just compare the single member inside it.’