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