Simplifying minimum search of two dimensional vector with complex struct
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
1
down vote
favorite
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);
c++ c++11 lambda
add a comment |Â
up vote
1
down vote
favorite
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);
c++ c++11 lambda
add a comment |Â
up vote
1
down vote
favorite
up vote
1
down vote
favorite
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);
c++ c++11 lambda
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);
c++ c++11 lambda
edited May 25 at 8:28
JDÃ Âugosz
5,047731
5,047731
asked May 25 at 8:17
Ben1980
156
156
add a comment |Â
add a comment |Â
2 Answers
2
active
oldest
votes
up vote
4
down vote
accepted
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
add a comment |Â
up vote
2
down vote
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.âÂÂ
add a comment |Â
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
4
down vote
accepted
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
add a comment |Â
up vote
4
down vote
accepted
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
add a comment |Â
up vote
4
down vote
accepted
up vote
4
down vote
accepted
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
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
edited May 25 at 11:57
answered May 25 at 10:08
papagaga
2,594115
2,594115
add a comment |Â
add a comment |Â
up vote
2
down vote
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.âÂÂ
add a comment |Â
up vote
2
down vote
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.âÂÂ
add a comment |Â
up vote
2
down vote
up vote
2
down vote
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.âÂÂ
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.âÂÂ
answered May 25 at 8:34
JDÃ Âugosz
5,047731
5,047731
add a comment |Â
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f195139%2fsimplifying-minimum-search-of-two-dimensional-vector-with-complex-struct%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password