Compare a given version number in the form major.minor.build.patch and see if one is less than the other Announcing the arrival of Valued Associate #679: Cesar Manara Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)Displaying the number of elements larger than the average of an arrayFirst prime number larger than given integerFind smallest prime number greater than given nPrinting twin primes less than a given natural number nFind how many numbers in an array are less than each number in the arraySimple pool of threads which calculate the sum of a given number from the main processCalculate the number of palindrome numbers in the given rangesA program to find out the number of odd and even Fibonacci numbers between given rangeFind the smallest number in the first array that is not in the second oneFind the sum of the digits of a given number

Can an alien society believe that their star system is the universe?

English words in a non-english sci-fi novel

Apollo command module space walk?

Echoing a tail command produces unexpected output?

What does the "x" in "x86" represent?

What is Arya's weapon design?

If a contract sometimes uses the wrong name, is it still valid?

How can I make names more distinctive without making them longer?

How do pianists reach extremely loud dynamics?

How do I name drop voicings

How to react to hostile behavior from a senior developer?

Why didn't this character "real die" when they blew their stack out in Altered Carbon?

Short Story with Cinderella as a Voo-doo Witch

Identifying polygons that intersect with another layer using QGIS?

Error "illegal generic type for instanceof" when using local classes

Seeking colloquialism for “just because”

Storing hydrofluoric acid before the invention of plastics

Why aren't air breathing engines used as small first stages

What does an IRS interview request entail when called in to verify expenses for a sole proprietor small business?

What is Wonderstone and are there any references to it pre-1982?

Ring Automorphisms that fix 1.

How to call a function with default parameter through a pointer to function that is the return of another function?

Can I cast Passwall to drop an enemy into a 20-foot pit?

Why is "Consequences inflicted." not a sentence?



Compare a given version number in the form major.minor.build.patch and see if one is less than the other



Announcing the arrival of Valued Associate #679: Cesar Manara
Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)Displaying the number of elements larger than the average of an arrayFirst prime number larger than given integerFind smallest prime number greater than given nPrinting twin primes less than a given natural number nFind how many numbers in an array are less than each number in the arraySimple pool of threads which calculate the sum of a given number from the main processCalculate the number of palindrome numbers in the given rangesA program to find out the number of odd and even Fibonacci numbers between given rangeFind the smallest number in the first array that is not in the second oneFind the sum of the digits of a given number



.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;








2












$begingroup$


#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>

typedef int STATUS;
#define ERROR -1
#define OKAY 0

struct version

unsigned char major;
unsigned char minor;
unsigned char build;
unsigned char patch;
;
STATUS is_less_than(struct version * original, struct version *compared, bool *result)

if(original == NULL


Is there a cleaner way to do this?










share|improve this question









$endgroup$


















    2












    $begingroup$


    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include <stdbool.h>

    typedef int STATUS;
    #define ERROR -1
    #define OKAY 0

    struct version

    unsigned char major;
    unsigned char minor;
    unsigned char build;
    unsigned char patch;
    ;
    STATUS is_less_than(struct version * original, struct version *compared, bool *result)

    if(original == NULL


    Is there a cleaner way to do this?










    share|improve this question









    $endgroup$














      2












      2








      2





      $begingroup$


      #include <stdio.h>
      #include <stdlib.h>
      #include <string.h>
      #include <stdbool.h>

      typedef int STATUS;
      #define ERROR -1
      #define OKAY 0

      struct version

      unsigned char major;
      unsigned char minor;
      unsigned char build;
      unsigned char patch;
      ;
      STATUS is_less_than(struct version * original, struct version *compared, bool *result)

      if(original == NULL


      Is there a cleaner way to do this?










      share|improve this question









      $endgroup$




      #include <stdio.h>
      #include <stdlib.h>
      #include <string.h>
      #include <stdbool.h>

      typedef int STATUS;
      #define ERROR -1
      #define OKAY 0

      struct version

      unsigned char major;
      unsigned char minor;
      unsigned char build;
      unsigned char patch;
      ;
      STATUS is_less_than(struct version * original, struct version *compared, bool *result)

      if(original == NULL


      Is there a cleaner way to do this?







      c






      share|improve this question













      share|improve this question











      share|improve this question




      share|improve this question










      asked 5 hours ago









      the_endianthe_endian

      396212




      396212




















          3 Answers
          3






          active

          oldest

          votes


















          2












          $begingroup$

          Yes, there is a cleaner way:



          if (a.major != b.major) 
          *result = a.major < b.major;
          else if (a.minor != b.minor)
          *result = a.minor < b.minor;
          else if (a.patch != b.patch)
          *result = a.patch < b.patch;
          else
          *result = a.build < b.build;

          return OKAY;


          I reordered patch to come before build since that's how it is usually done. If your version scheme is different from this, good luck.



          Instead of unsigned char I would choose unsigned int so that your code can handle versions like 1.0.20190415.






          share|improve this answer











          $endgroup$












          • $begingroup$
            Nice catch on the patch, build ordering.
            $endgroup$
            – Costantino Grana
            1 hour ago


















          0












          $begingroup$

          Return status



          You create this:



          typedef int STATUS;
          #define ERROR -1
          #define OKAY 0


          which is basically a boolean status. Personally, I'd return a straight bool.



          Bug/Not what you mean



          Doing a



          result = NULL;


          is changing the local variable (parameter) result. It's not setting the result to NULL. In fact the caller won't probably have a pointer at all, but just a bool, which cannot properly be NULL.



          Shorter version



          I'm not sure this is cleaner, but here I go:



          bool is_less_than(struct version * original, struct version *compared, bool *result)
          compared == NULL


          Next time, add a driver/test suite to your question, to ease the life of people answering. This can be one:



          int main(void) 

          struct version ref = 1, 2, 21, 8 ;
          struct version lower1 = 0, 2, 21, 8 ;
          struct version lower2 = 1, 1, 21, 8 ;
          struct version lower3 = 1, 2, 20, 8 ;
          struct version lower4 = 1, 2, 21, 7 ;
          struct version equal = 1, 2, 21, 8 ;
          struct version higher1 = 2, 2, 21, 8 ;
          struct version higher2 = 1, 3, 21, 8 ;
          struct version higher3 = 1, 2, 22, 8 ;
          struct version higher4 = 1, 2, 21, 9 ;

          #define TEST(a,b,expect1,expect2)
          do
          bool result1, result2;
          is_less_than((a), (b), &result1);
          is_less_than((b), (a), &result2);
          puts(result1==(expect1) && result2==(expect2)?"ok":"failed");
          while(0)
          #define TESTL(a,b) TEST(a,b,true,false)
          #define TESTE(a,b) TEST(a,b,false,false)
          #define TESTH(a,b) TEST(a,b,false,true)

          TESTL(&lower1, &ref);
          TESTL(&lower2, &ref);
          TESTL(&lower3, &ref);
          TESTL(&lower4, &ref);
          TESTE(&equal, &ref);
          TESTH(&higher1, &ref);
          TESTH(&higher2, &ref);
          TESTH(&higher3, &ref);
          TESTH(&higher4, &ref);

          return 0;






          share|improve this answer











          $endgroup$








          • 1




            $begingroup$
            As for every comparator function, the driver/test should compare each pair of example data to at least ensure that the ordering is transitive and that less(x, x) is never true.
            $endgroup$
            – Roland Illig
            1 hour ago











          • $begingroup$
            @RolandIllig Updated. Thank you for the suggestion.
            $endgroup$
            – Costantino Grana
            59 mins ago


















          0












          $begingroup$

          I don't see any advantage to having the function to take three pointers (two for input and one for output) and return a status code. As a result of that unnecessarily error-prone design, the function has to handle the possibility of null pointers, and the caller is expected to handle a status code. But why should such a simple comparison have these failure modes at all?



          The danger is further complicated by the fact that neither of the in-parameters is declared const.



          Just pass the two versions by value, and you would eliminate all of that complication! On any modern 32-bit or 64-bit processor, passing a four-byte struct by value should actually be more efficient than passing it by reference — especially since you don't have to dereference the pointers to access each field.



          With all of the potential errors out of the way, taking @RolandIllig's suggestion, you could then reduce it down to one chained conditional expression:



          bool is_less_than(struct version a, struct version b) 
          return a.major != b.major ? a.major < b.major :
          a.minor != b.minor ? a.minor < b.minor :
          a.patch != b.patch ? a.patch < b.patch :
          a.build < b.build;



          I'd go further and recommend using unsigned short instead of unsigned char for the fields. Using unsigned char for numeric values is awkward, since you would have to cast them when using printf(). On a 64-bit architecture, a struct with four 2-byte fields would occupy 64 bits, so you wouldn't be saving anything by using unsigned char instead of unsigned short.






          share|improve this answer









          $endgroup$













            Your Answer






            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',
            autoActivateHeartbeat: false,
            convertImagesToLinks: false,
            noModals: true,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            imageUploader:
            brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
            contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
            allowUrls: true
            ,
            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%2f217587%2fcompare-a-given-version-number-in-the-form-major-minor-build-patch-and-see-if-on%23new-answer', 'question_page');

            );

            Post as a guest















            Required, but never shown

























            3 Answers
            3






            active

            oldest

            votes








            3 Answers
            3






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes









            2












            $begingroup$

            Yes, there is a cleaner way:



            if (a.major != b.major) 
            *result = a.major < b.major;
            else if (a.minor != b.minor)
            *result = a.minor < b.minor;
            else if (a.patch != b.patch)
            *result = a.patch < b.patch;
            else
            *result = a.build < b.build;

            return OKAY;


            I reordered patch to come before build since that's how it is usually done. If your version scheme is different from this, good luck.



            Instead of unsigned char I would choose unsigned int so that your code can handle versions like 1.0.20190415.






            share|improve this answer











            $endgroup$












            • $begingroup$
              Nice catch on the patch, build ordering.
              $endgroup$
              – Costantino Grana
              1 hour ago















            2












            $begingroup$

            Yes, there is a cleaner way:



            if (a.major != b.major) 
            *result = a.major < b.major;
            else if (a.minor != b.minor)
            *result = a.minor < b.minor;
            else if (a.patch != b.patch)
            *result = a.patch < b.patch;
            else
            *result = a.build < b.build;

            return OKAY;


            I reordered patch to come before build since that's how it is usually done. If your version scheme is different from this, good luck.



            Instead of unsigned char I would choose unsigned int so that your code can handle versions like 1.0.20190415.






            share|improve this answer











            $endgroup$












            • $begingroup$
              Nice catch on the patch, build ordering.
              $endgroup$
              – Costantino Grana
              1 hour ago













            2












            2








            2





            $begingroup$

            Yes, there is a cleaner way:



            if (a.major != b.major) 
            *result = a.major < b.major;
            else if (a.minor != b.minor)
            *result = a.minor < b.minor;
            else if (a.patch != b.patch)
            *result = a.patch < b.patch;
            else
            *result = a.build < b.build;

            return OKAY;


            I reordered patch to come before build since that's how it is usually done. If your version scheme is different from this, good luck.



            Instead of unsigned char I would choose unsigned int so that your code can handle versions like 1.0.20190415.






            share|improve this answer











            $endgroup$



            Yes, there is a cleaner way:



            if (a.major != b.major) 
            *result = a.major < b.major;
            else if (a.minor != b.minor)
            *result = a.minor < b.minor;
            else if (a.patch != b.patch)
            *result = a.patch < b.patch;
            else
            *result = a.build < b.build;

            return OKAY;


            I reordered patch to come before build since that's how it is usually done. If your version scheme is different from this, good luck.



            Instead of unsigned char I would choose unsigned int so that your code can handle versions like 1.0.20190415.







            share|improve this answer














            share|improve this answer



            share|improve this answer








            edited 1 hour ago

























            answered 1 hour ago









            Roland IlligRoland Illig

            11.6k11946




            11.6k11946











            • $begingroup$
              Nice catch on the patch, build ordering.
              $endgroup$
              – Costantino Grana
              1 hour ago
















            • $begingroup$
              Nice catch on the patch, build ordering.
              $endgroup$
              – Costantino Grana
              1 hour ago















            $begingroup$
            Nice catch on the patch, build ordering.
            $endgroup$
            – Costantino Grana
            1 hour ago




            $begingroup$
            Nice catch on the patch, build ordering.
            $endgroup$
            – Costantino Grana
            1 hour ago













            0












            $begingroup$

            Return status



            You create this:



            typedef int STATUS;
            #define ERROR -1
            #define OKAY 0


            which is basically a boolean status. Personally, I'd return a straight bool.



            Bug/Not what you mean



            Doing a



            result = NULL;


            is changing the local variable (parameter) result. It's not setting the result to NULL. In fact the caller won't probably have a pointer at all, but just a bool, which cannot properly be NULL.



            Shorter version



            I'm not sure this is cleaner, but here I go:



            bool is_less_than(struct version * original, struct version *compared, bool *result)
            compared == NULL


            Next time, add a driver/test suite to your question, to ease the life of people answering. This can be one:



            int main(void) 

            struct version ref = 1, 2, 21, 8 ;
            struct version lower1 = 0, 2, 21, 8 ;
            struct version lower2 = 1, 1, 21, 8 ;
            struct version lower3 = 1, 2, 20, 8 ;
            struct version lower4 = 1, 2, 21, 7 ;
            struct version equal = 1, 2, 21, 8 ;
            struct version higher1 = 2, 2, 21, 8 ;
            struct version higher2 = 1, 3, 21, 8 ;
            struct version higher3 = 1, 2, 22, 8 ;
            struct version higher4 = 1, 2, 21, 9 ;

            #define TEST(a,b,expect1,expect2)
            do
            bool result1, result2;
            is_less_than((a), (b), &result1);
            is_less_than((b), (a), &result2);
            puts(result1==(expect1) && result2==(expect2)?"ok":"failed");
            while(0)
            #define TESTL(a,b) TEST(a,b,true,false)
            #define TESTE(a,b) TEST(a,b,false,false)
            #define TESTH(a,b) TEST(a,b,false,true)

            TESTL(&lower1, &ref);
            TESTL(&lower2, &ref);
            TESTL(&lower3, &ref);
            TESTL(&lower4, &ref);
            TESTE(&equal, &ref);
            TESTH(&higher1, &ref);
            TESTH(&higher2, &ref);
            TESTH(&higher3, &ref);
            TESTH(&higher4, &ref);

            return 0;






            share|improve this answer











            $endgroup$








            • 1




              $begingroup$
              As for every comparator function, the driver/test should compare each pair of example data to at least ensure that the ordering is transitive and that less(x, x) is never true.
              $endgroup$
              – Roland Illig
              1 hour ago











            • $begingroup$
              @RolandIllig Updated. Thank you for the suggestion.
              $endgroup$
              – Costantino Grana
              59 mins ago















            0












            $begingroup$

            Return status



            You create this:



            typedef int STATUS;
            #define ERROR -1
            #define OKAY 0


            which is basically a boolean status. Personally, I'd return a straight bool.



            Bug/Not what you mean



            Doing a



            result = NULL;


            is changing the local variable (parameter) result. It's not setting the result to NULL. In fact the caller won't probably have a pointer at all, but just a bool, which cannot properly be NULL.



            Shorter version



            I'm not sure this is cleaner, but here I go:



            bool is_less_than(struct version * original, struct version *compared, bool *result)
            compared == NULL


            Next time, add a driver/test suite to your question, to ease the life of people answering. This can be one:



            int main(void) 

            struct version ref = 1, 2, 21, 8 ;
            struct version lower1 = 0, 2, 21, 8 ;
            struct version lower2 = 1, 1, 21, 8 ;
            struct version lower3 = 1, 2, 20, 8 ;
            struct version lower4 = 1, 2, 21, 7 ;
            struct version equal = 1, 2, 21, 8 ;
            struct version higher1 = 2, 2, 21, 8 ;
            struct version higher2 = 1, 3, 21, 8 ;
            struct version higher3 = 1, 2, 22, 8 ;
            struct version higher4 = 1, 2, 21, 9 ;

            #define TEST(a,b,expect1,expect2)
            do
            bool result1, result2;
            is_less_than((a), (b), &result1);
            is_less_than((b), (a), &result2);
            puts(result1==(expect1) && result2==(expect2)?"ok":"failed");
            while(0)
            #define TESTL(a,b) TEST(a,b,true,false)
            #define TESTE(a,b) TEST(a,b,false,false)
            #define TESTH(a,b) TEST(a,b,false,true)

            TESTL(&lower1, &ref);
            TESTL(&lower2, &ref);
            TESTL(&lower3, &ref);
            TESTL(&lower4, &ref);
            TESTE(&equal, &ref);
            TESTH(&higher1, &ref);
            TESTH(&higher2, &ref);
            TESTH(&higher3, &ref);
            TESTH(&higher4, &ref);

            return 0;






            share|improve this answer











            $endgroup$








            • 1




              $begingroup$
              As for every comparator function, the driver/test should compare each pair of example data to at least ensure that the ordering is transitive and that less(x, x) is never true.
              $endgroup$
              – Roland Illig
              1 hour ago











            • $begingroup$
              @RolandIllig Updated. Thank you for the suggestion.
              $endgroup$
              – Costantino Grana
              59 mins ago













            0












            0








            0





            $begingroup$

            Return status



            You create this:



            typedef int STATUS;
            #define ERROR -1
            #define OKAY 0


            which is basically a boolean status. Personally, I'd return a straight bool.



            Bug/Not what you mean



            Doing a



            result = NULL;


            is changing the local variable (parameter) result. It's not setting the result to NULL. In fact the caller won't probably have a pointer at all, but just a bool, which cannot properly be NULL.



            Shorter version



            I'm not sure this is cleaner, but here I go:



            bool is_less_than(struct version * original, struct version *compared, bool *result)
            compared == NULL


            Next time, add a driver/test suite to your question, to ease the life of people answering. This can be one:



            int main(void) 

            struct version ref = 1, 2, 21, 8 ;
            struct version lower1 = 0, 2, 21, 8 ;
            struct version lower2 = 1, 1, 21, 8 ;
            struct version lower3 = 1, 2, 20, 8 ;
            struct version lower4 = 1, 2, 21, 7 ;
            struct version equal = 1, 2, 21, 8 ;
            struct version higher1 = 2, 2, 21, 8 ;
            struct version higher2 = 1, 3, 21, 8 ;
            struct version higher3 = 1, 2, 22, 8 ;
            struct version higher4 = 1, 2, 21, 9 ;

            #define TEST(a,b,expect1,expect2)
            do
            bool result1, result2;
            is_less_than((a), (b), &result1);
            is_less_than((b), (a), &result2);
            puts(result1==(expect1) && result2==(expect2)?"ok":"failed");
            while(0)
            #define TESTL(a,b) TEST(a,b,true,false)
            #define TESTE(a,b) TEST(a,b,false,false)
            #define TESTH(a,b) TEST(a,b,false,true)

            TESTL(&lower1, &ref);
            TESTL(&lower2, &ref);
            TESTL(&lower3, &ref);
            TESTL(&lower4, &ref);
            TESTE(&equal, &ref);
            TESTH(&higher1, &ref);
            TESTH(&higher2, &ref);
            TESTH(&higher3, &ref);
            TESTH(&higher4, &ref);

            return 0;






            share|improve this answer











            $endgroup$



            Return status



            You create this:



            typedef int STATUS;
            #define ERROR -1
            #define OKAY 0


            which is basically a boolean status. Personally, I'd return a straight bool.



            Bug/Not what you mean



            Doing a



            result = NULL;


            is changing the local variable (parameter) result. It's not setting the result to NULL. In fact the caller won't probably have a pointer at all, but just a bool, which cannot properly be NULL.



            Shorter version



            I'm not sure this is cleaner, but here I go:



            bool is_less_than(struct version * original, struct version *compared, bool *result)
            compared == NULL


            Next time, add a driver/test suite to your question, to ease the life of people answering. This can be one:



            int main(void) 

            struct version ref = 1, 2, 21, 8 ;
            struct version lower1 = 0, 2, 21, 8 ;
            struct version lower2 = 1, 1, 21, 8 ;
            struct version lower3 = 1, 2, 20, 8 ;
            struct version lower4 = 1, 2, 21, 7 ;
            struct version equal = 1, 2, 21, 8 ;
            struct version higher1 = 2, 2, 21, 8 ;
            struct version higher2 = 1, 3, 21, 8 ;
            struct version higher3 = 1, 2, 22, 8 ;
            struct version higher4 = 1, 2, 21, 9 ;

            #define TEST(a,b,expect1,expect2)
            do
            bool result1, result2;
            is_less_than((a), (b), &result1);
            is_less_than((b), (a), &result2);
            puts(result1==(expect1) && result2==(expect2)?"ok":"failed");
            while(0)
            #define TESTL(a,b) TEST(a,b,true,false)
            #define TESTE(a,b) TEST(a,b,false,false)
            #define TESTH(a,b) TEST(a,b,false,true)

            TESTL(&lower1, &ref);
            TESTL(&lower2, &ref);
            TESTL(&lower3, &ref);
            TESTL(&lower4, &ref);
            TESTE(&equal, &ref);
            TESTH(&higher1, &ref);
            TESTH(&higher2, &ref);
            TESTH(&higher3, &ref);
            TESTH(&higher4, &ref);

            return 0;







            share|improve this answer














            share|improve this answer



            share|improve this answer








            edited 1 hour ago

























            answered 1 hour ago









            Costantino GranaCostantino Grana

            18728




            18728







            • 1




              $begingroup$
              As for every comparator function, the driver/test should compare each pair of example data to at least ensure that the ordering is transitive and that less(x, x) is never true.
              $endgroup$
              – Roland Illig
              1 hour ago











            • $begingroup$
              @RolandIllig Updated. Thank you for the suggestion.
              $endgroup$
              – Costantino Grana
              59 mins ago












            • 1




              $begingroup$
              As for every comparator function, the driver/test should compare each pair of example data to at least ensure that the ordering is transitive and that less(x, x) is never true.
              $endgroup$
              – Roland Illig
              1 hour ago











            • $begingroup$
              @RolandIllig Updated. Thank you for the suggestion.
              $endgroup$
              – Costantino Grana
              59 mins ago







            1




            1




            $begingroup$
            As for every comparator function, the driver/test should compare each pair of example data to at least ensure that the ordering is transitive and that less(x, x) is never true.
            $endgroup$
            – Roland Illig
            1 hour ago





            $begingroup$
            As for every comparator function, the driver/test should compare each pair of example data to at least ensure that the ordering is transitive and that less(x, x) is never true.
            $endgroup$
            – Roland Illig
            1 hour ago













            $begingroup$
            @RolandIllig Updated. Thank you for the suggestion.
            $endgroup$
            – Costantino Grana
            59 mins ago




            $begingroup$
            @RolandIllig Updated. Thank you for the suggestion.
            $endgroup$
            – Costantino Grana
            59 mins ago











            0












            $begingroup$

            I don't see any advantage to having the function to take three pointers (two for input and one for output) and return a status code. As a result of that unnecessarily error-prone design, the function has to handle the possibility of null pointers, and the caller is expected to handle a status code. But why should such a simple comparison have these failure modes at all?



            The danger is further complicated by the fact that neither of the in-parameters is declared const.



            Just pass the two versions by value, and you would eliminate all of that complication! On any modern 32-bit or 64-bit processor, passing a four-byte struct by value should actually be more efficient than passing it by reference — especially since you don't have to dereference the pointers to access each field.



            With all of the potential errors out of the way, taking @RolandIllig's suggestion, you could then reduce it down to one chained conditional expression:



            bool is_less_than(struct version a, struct version b) 
            return a.major != b.major ? a.major < b.major :
            a.minor != b.minor ? a.minor < b.minor :
            a.patch != b.patch ? a.patch < b.patch :
            a.build < b.build;



            I'd go further and recommend using unsigned short instead of unsigned char for the fields. Using unsigned char for numeric values is awkward, since you would have to cast them when using printf(). On a 64-bit architecture, a struct with four 2-byte fields would occupy 64 bits, so you wouldn't be saving anything by using unsigned char instead of unsigned short.






            share|improve this answer









            $endgroup$

















              0












              $begingroup$

              I don't see any advantage to having the function to take three pointers (two for input and one for output) and return a status code. As a result of that unnecessarily error-prone design, the function has to handle the possibility of null pointers, and the caller is expected to handle a status code. But why should such a simple comparison have these failure modes at all?



              The danger is further complicated by the fact that neither of the in-parameters is declared const.



              Just pass the two versions by value, and you would eliminate all of that complication! On any modern 32-bit or 64-bit processor, passing a four-byte struct by value should actually be more efficient than passing it by reference — especially since you don't have to dereference the pointers to access each field.



              With all of the potential errors out of the way, taking @RolandIllig's suggestion, you could then reduce it down to one chained conditional expression:



              bool is_less_than(struct version a, struct version b) 
              return a.major != b.major ? a.major < b.major :
              a.minor != b.minor ? a.minor < b.minor :
              a.patch != b.patch ? a.patch < b.patch :
              a.build < b.build;



              I'd go further and recommend using unsigned short instead of unsigned char for the fields. Using unsigned char for numeric values is awkward, since you would have to cast them when using printf(). On a 64-bit architecture, a struct with four 2-byte fields would occupy 64 bits, so you wouldn't be saving anything by using unsigned char instead of unsigned short.






              share|improve this answer









              $endgroup$















                0












                0








                0





                $begingroup$

                I don't see any advantage to having the function to take three pointers (two for input and one for output) and return a status code. As a result of that unnecessarily error-prone design, the function has to handle the possibility of null pointers, and the caller is expected to handle a status code. But why should such a simple comparison have these failure modes at all?



                The danger is further complicated by the fact that neither of the in-parameters is declared const.



                Just pass the two versions by value, and you would eliminate all of that complication! On any modern 32-bit or 64-bit processor, passing a four-byte struct by value should actually be more efficient than passing it by reference — especially since you don't have to dereference the pointers to access each field.



                With all of the potential errors out of the way, taking @RolandIllig's suggestion, you could then reduce it down to one chained conditional expression:



                bool is_less_than(struct version a, struct version b) 
                return a.major != b.major ? a.major < b.major :
                a.minor != b.minor ? a.minor < b.minor :
                a.patch != b.patch ? a.patch < b.patch :
                a.build < b.build;



                I'd go further and recommend using unsigned short instead of unsigned char for the fields. Using unsigned char for numeric values is awkward, since you would have to cast them when using printf(). On a 64-bit architecture, a struct with four 2-byte fields would occupy 64 bits, so you wouldn't be saving anything by using unsigned char instead of unsigned short.






                share|improve this answer









                $endgroup$



                I don't see any advantage to having the function to take three pointers (two for input and one for output) and return a status code. As a result of that unnecessarily error-prone design, the function has to handle the possibility of null pointers, and the caller is expected to handle a status code. But why should such a simple comparison have these failure modes at all?



                The danger is further complicated by the fact that neither of the in-parameters is declared const.



                Just pass the two versions by value, and you would eliminate all of that complication! On any modern 32-bit or 64-bit processor, passing a four-byte struct by value should actually be more efficient than passing it by reference — especially since you don't have to dereference the pointers to access each field.



                With all of the potential errors out of the way, taking @RolandIllig's suggestion, you could then reduce it down to one chained conditional expression:



                bool is_less_than(struct version a, struct version b) 
                return a.major != b.major ? a.major < b.major :
                a.minor != b.minor ? a.minor < b.minor :
                a.patch != b.patch ? a.patch < b.patch :
                a.build < b.build;



                I'd go further and recommend using unsigned short instead of unsigned char for the fields. Using unsigned char for numeric values is awkward, since you would have to cast them when using printf(). On a 64-bit architecture, a struct with four 2-byte fields would occupy 64 bits, so you wouldn't be saving anything by using unsigned char instead of unsigned short.







                share|improve this answer












                share|improve this answer



                share|improve this answer










                answered 27 mins ago









                200_success200_success

                131k17157422




                131k17157422



























                    draft saved

                    draft discarded
















































                    Thanks for contributing an answer to Code Review Stack Exchange!


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid


                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.

                    Use MathJax to format equations. MathJax reference.


                    To learn more, see our tips on writing great answers.




                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function ()
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f217587%2fcompare-a-given-version-number-in-the-form-major-minor-build-patch-and-see-if-on%23new-answer', 'question_page');

                    );

                    Post as a guest















                    Required, but never shown





















































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown

































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown







                    Popular posts from this blog

                    Are there any AGPL-style licences that require source code modifications to be public? Planned maintenance scheduled April 23, 2019 at 23:30 UTC (7:30pm US/Eastern) Announcing the arrival of Valued Associate #679: Cesar Manara Unicorn Meta Zoo #1: Why another podcast?Force derivative works to be publicAre there any GPL like licenses for Apple App Store?Do you violate the GPL if you provide source code that cannot be compiled?GPL - is it distribution to use libraries in an appliance loaned to customers?Distributing App for free which uses GPL'ed codeModifications of server software under GPL, with web/CLI interfaceDoes using an AGPLv3-licensed library prevent me from dual-licensing my own source code?Can I publish only select code under GPLv3 from a private project?Is there published precedent regarding the scope of covered work that uses AGPL software?If MIT licensed code links to GPL licensed code what should be the license of the resulting binary program?If I use a public API endpoint that has its source code licensed under AGPL in my app, do I need to disclose my source?

                    2013 GY136 Descoberta | Órbita | Referências Menu de navegação«List Of Centaurs and Scattered-Disk Objects»«List of Known Trans-Neptunian Objects»

                    Button changing it's text & action. Good or terrible? The 2019 Stack Overflow Developer Survey Results Are Inchanging text on user mouseoverShould certain functions be “hard to find” for powerusers to discover?Custom liking function - do I need user login?Using different checkbox style for different checkbox behaviorBest Practices: Save and Exit in Software UIInteraction with remote validated formMore efficient UI to progress the user through a complicated process?Designing a popup notice for a gameShould bulk-editing functions be hidden until a table row is selected, or is there a better solution?Is it bad practice to disable (replace) the context menu?