- 145.5k
- 22
- 190
- 478
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
. It's also entirely conceivable that you might need mire than 256 builds within a patch level.
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
.
I don't see any advantage to having the function 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
. It's also entirely conceivable that you might need mire than 256 builds within a patch level.
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
.