Simplifying minimum search of two dimensional vector with complex struct

The name of the pictureThe name of the pictureThe name of the pictureClash 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);






share|improve this question



























    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);






    share|improve this question























      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);






      share|improve this 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);








      share|improve this question












      share|improve this question




      share|improve this question








      edited May 25 at 8:28









      JDługosz

      5,047731




      5,047731









      asked May 25 at 8:17









      Ben1980

      156




      156




















          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






          share|improve this answer






























            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.’






            share|improve this answer





















              Your Answer




              StackExchange.ifUsing("editor", function ()
              return StackExchange.using("mathjaxEditing", function ()
              StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
              StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
              );
              );
              , "mathjax-editing");

              StackExchange.ifUsing("editor", function ()
              StackExchange.using("externalEditor", function ()
              StackExchange.using("snippets", function ()
              StackExchange.snippets.init();
              );
              );
              , "code-snippets");

              StackExchange.ready(function()
              var channelOptions =
              tags: "".split(" "),
              id: "196"
              ;
              initTagRenderer("".split(" "), "".split(" "), channelOptions);

              StackExchange.using("externalEditor", function()
              // Have to fire editor after snippets, if snippets enabled
              if (StackExchange.settings.snippets.snippetsEnabled)
              StackExchange.using("snippets", function()
              createEditor();
              );

              else
              createEditor();

              );

              function createEditor()
              StackExchange.prepareEditor(
              heartbeatType: 'answer',
              convertImagesToLinks: false,
              noModals: false,
              showLowRepImageUploadWarning: true,
              reputationToPostImages: null,
              bindNavPrevention: true,
              postfix: "",
              onDemand: true,
              discardSelector: ".discard-answer"
              ,immediatelyShowMarkdownHelp:true
              );



              );








               

              draft saved


              draft discarded


















              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






























              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






              share|improve this answer



























                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






                share|improve this answer

























                  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






                  share|improve this answer















                  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







                  share|improve this answer















                  share|improve this answer



                  share|improve this answer








                  edited May 25 at 11:57


























                  answered May 25 at 10:08









                  papagaga

                  2,594115




                  2,594115






















                      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.’






                      share|improve this answer

























                        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.’






                        share|improve this answer























                          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.’






                          share|improve this answer













                          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.’







                          share|improve this answer













                          share|improve this answer



                          share|improve this answer











                          answered May 25 at 8:34









                          JDługosz

                          5,047731




                          5,047731






















                               

                              draft saved


                              draft discarded


























                               


                              draft saved


                              draft discarded














                              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













































































                              Popular posts from this blog

                              Greedy Best First Search implementation in Rust

                              Function to Return a JSON Like Objects Using VBA Collections and Arrays

                              C++11 CLH Lock Implementation