Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

As far as the C programming language goes, check_is_numeric is a quite good function name. I could spend some time talking about a few variable names through your code that could stand to be a little bit better (as people around here know, I'm a stickler for writing readable code).

But we can also use types as a way to improve the readability and self-documentation of our code base.

Your check_is_numeric function has a return type of int. Based on reading the function name, I'd assume a bool return type: true or false (and be disappointed that you're not using a bool). There's no comments to go along with the function, and I'm not sure where I'd find any technical documentation. So, it might be extraordinarily easy for me to miss the fact that your function even differentiates between integer and floating point numbers. I could very easily have written the following...

if (check_is_numeric(3.14159)) {

And I'd never notice that this is returning 2, which will still behave as expected here if we are working under the assumption that the function simply returns true or false.

Even if we do spend some time playing with the function and see that we sometimes get 1 and sometimes get 2 and sometimes get 0, it might be difficult to notice what these values mean (especially if there are some bugs bugs). So, importantly, I could incorrectly make the assumption that your function returns 0 for non-numbers, and non-zero for numbers, and that number returned is insignificant.

Or we could make our code self-documenting with an enum which clearly self-describes what the return value means:

typedef enum { 
 NON_NUMBER, 
 INTEGER_NUMBER, 
 FLOATING_POINT_NUMBER
} number_type;

And now, our function returns this type:

number_type check_is_numeric(const char * string) {
 // ...
 return NON_NUMBER;
 // or...
 return INTEGER_NUMBER;
 // or...
 return FLOATING_POINT_NUMBER;
}

As far as the C programming language goes, check_is_numeric is a quite good function name. I could spend some time talking about a few variable names through your code that could stand to be a little bit better (as people around here know, I'm a stickler for writing readable code).

But we can also use types as a way to improve the readability and self-documentation of our code base.

Your check_is_numeric function has a return type of int. Based on reading the function name, I'd assume a bool return type: true or false (and be disappointed that you're not using a bool). There's no comments to go along with the function, and I'm not sure where I'd find any technical documentation. So, it might be extraordinarily easy for me to miss the fact that your function even differentiates between integer and floating point numbers. I could very easily have written the following...

if (check_is_numeric(3.14159)) {

And I'd never notice that this is returning 2, which will still behave as expected here if we are working under the assumption that the function simply returns true or false.

Even if we do spend some time playing with the function and see that we sometimes get 1 and sometimes get 2 and sometimes get 0, it might be difficult to notice what these values mean (especially if there are some bugs). So, importantly, I could incorrectly make the assumption that your function returns 0 for non-numbers, and non-zero for numbers, and that number returned is insignificant.

Or we could make our code self-documenting with an enum which clearly self-describes what the return value means:

typedef enum { 
 NON_NUMBER, 
 INTEGER_NUMBER, 
 FLOATING_POINT_NUMBER
} number_type;

And now, our function returns this type:

number_type check_is_numeric(const char * string) {
 // ...
 return NON_NUMBER;
 // or...
 return INTEGER_NUMBER;
 // or...
 return FLOATING_POINT_NUMBER;
}

As far as the C programming language goes, check_is_numeric is a quite good function name. I could spend some time talking about a few variable names through your code that could stand to be a little bit better (as people around here know, I'm a stickler for writing readable code).

But we can also use types as a way to improve the readability and self-documentation of our code base.

Your check_is_numeric function has a return type of int. Based on reading the function name, I'd assume a bool return type: true or false (and be disappointed that you're not using a bool). There's no comments to go along with the function, and I'm not sure where I'd find any technical documentation. So, it might be extraordinarily easy for me to miss the fact that your function even differentiates between integer and floating point numbers. I could very easily have written the following...

if (check_is_numeric(3.14159)) {

And I'd never notice that this is returning 2, which will still behave as expected here if we are working under the assumption that the function simply returns true or false.

Even if we do spend some time playing with the function and see that we sometimes get 1 and sometimes get 2 and sometimes get 0, it might be difficult to notice what these values mean (especially if there are some bugs). So, importantly, I could incorrectly make the assumption that your function returns 0 for non-numbers, and non-zero for numbers, and that number returned is insignificant.

Or we could make our code self-documenting with an enum which clearly self-describes what the return value means:

typedef enum { 
 NON_NUMBER, 
 INTEGER_NUMBER, 
 FLOATING_POINT_NUMBER
} number_type;

And now, our function returns this type:

number_type check_is_numeric(const char * string) {
 // ...
 return NON_NUMBER;
 // or...
 return INTEGER_NUMBER;
 // or...
 return FLOATING_POINT_NUMBER;
}
Source Link
nhgrif
  • 25.4k
  • 3
  • 64
  • 129

As far as the C programming language goes, check_is_numeric is a quite good function name. I could spend some time talking about a few variable names through your code that could stand to be a little bit better (as people around here know, I'm a stickler for writing readable code).

But we can also use types as a way to improve the readability and self-documentation of our code base.

Your check_is_numeric function has a return type of int. Based on reading the function name, I'd assume a bool return type: true or false (and be disappointed that you're not using a bool). There's no comments to go along with the function, and I'm not sure where I'd find any technical documentation. So, it might be extraordinarily easy for me to miss the fact that your function even differentiates between integer and floating point numbers. I could very easily have written the following...

if (check_is_numeric(3.14159)) {

And I'd never notice that this is returning 2, which will still behave as expected here if we are working under the assumption that the function simply returns true or false.

Even if we do spend some time playing with the function and see that we sometimes get 1 and sometimes get 2 and sometimes get 0, it might be difficult to notice what these values mean (especially if there are some bugs). So, importantly, I could incorrectly make the assumption that your function returns 0 for non-numbers, and non-zero for numbers, and that number returned is insignificant.

Or we could make our code self-documenting with an enum which clearly self-describes what the return value means:

typedef enum { 
 NON_NUMBER, 
 INTEGER_NUMBER, 
 FLOATING_POINT_NUMBER
} number_type;

And now, our function returns this type:

number_type check_is_numeric(const char * string) {
 // ...
 return NON_NUMBER;
 // or...
 return INTEGER_NUMBER;
 // or...
 return FLOATING_POINT_NUMBER;
}
lang-c

AltStyle によって変換されたページ (->オリジナル) /