HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Numeric integrity check

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
checkintegritynumeric

Problem

I'm a beginning C programmer. I need a function that will take a string and determine if it's a number. The data is coming from files created by different people and the number formats WILL vary. So the function needs to decide if it was Supposed to be a number and if so, I will call the right conversion function (atoi/strtoi, atol/strtol etc etc.) So it needs to consider ints and floats, some with dollar signs, some with commas, some not. If a human would recognize it as a number the function should too.

The function works exactly as I want it to, and I haven't been able to break it no matter what I throw at it, but I'm sure there are better ways to do the job, and I'm equally sure there are problems with the way I'm doing it that I just am too inexperienced to know about yet. So Please, take a look and let me know of any potential pitfalls I may encounter with this function, feel free to suggest alternative ways if you're inclined.

Here's what I have come up with. As I've said it works, the example takes one or more command line args to simplify testing and there is no error checking yet, I will add that to the working program, but for function testing in a controlled setting I'm looking purely for functionality and problems that I might not know about with the way I'm handling the function.

Since the $ is significant to the shell it has to be enclosed in quotes but that won't be a problem when implemented because the data won't be coming from the command line.

Anything I should be aware of, or should consider with this code? More efficient mechanisms for accomplishing this would be appreciated. If there is a library function for this it has escaped my detection but would love to pointed in the right direction.

```
#include
#include
#include

int check_is_numeric(const char *arr)
{
//define valid characters for any numeric value
char numdig[] = {'0', '1', '2', '3', '4', '5',
'6', '7', '8', '9', ',', '.', '$'};
int j=0;
int notnum =

Solution

-
Using strtol() and strtod() would be a preferable way to determine if a string converts to a number. By using various locale settings, code can work with thousands separators too. This is a advanced task for a learner, so we will stick with your code.

-
Avoid magic numbers. Rather than code 13, use a derived limit.

// for(int b = 0; b < 13;b++){
for(int b = 0; b < sizeof numdig / sizeof numdig[0] ;b++){


-
Avoid negated variable names. Rather than notnum --> valid_num

-
For boolean variables like notnum/valid_num, use tpye bool. Be sure to include stdbool.h

-
Use standard functions like strspn(). This replaces the entire while loop. Make numdig[] both static and const for better optimization and greater clarity as to its use.

static const char numdig[] = {"0123456789,$."};
bool hasdec = false;
bool valid_num = arr[strspn(arr, numdig)] == '\0';

// search for dp separate
if (valid_num) {
  char *p = strchr(arr, '.');
  // dp found, see if another exist.
  if (p) {
    hasdec = true;
    valid_num = strchr(p+1, '.') == NULL;
  }
}


-
Good use of const in function signature.

int check_is_numeric(const char *arr)


-
Surprised signs '-' and '+' were not allowed.

-
Surprised optional leading whitespace was not allowed; very idiomatic in C.

-
Tests. I would have like to see the coding goals specifics and some of your sample test data to assess how thorough the claim "function works exactly as I want it to". I suspect input like "1,234,,456.0" will pass.

-
Advanced: '.' as decimal point and ',' as thousands separators is locale dependent. Research ` for details to make a function that works outside the "C" locale.

char dp = localeconv()->decimal_point[0];


-
Code looks like it ends with no
return. Drop else. Avoid !`.

if(valid_num) {
   if(hasdec) return 2;
   return 1;
 }
 return 0;

Code Snippets

// for(int b = 0; b < 13;b++){
for(int b = 0; b < sizeof numdig / sizeof numdig[0] ;b++){
static const char numdig[] = {"0123456789,$."};
bool hasdec = false;
bool valid_num = arr[strspn(arr, numdig)] == '\0';

// search for dp separate
if (valid_num) {
  char *p = strchr(arr, '.');
  // dp found, see if another exist.
  if (p) {
    hasdec = true;
    valid_num = strchr(p+1, '.') == NULL;
  }
}
int check_is_numeric(const char *arr)
char dp = localeconv()->decimal_point[0];
if(valid_num) {
   if(hasdec) return 2;
   return 1;
 }
 return 0;

Context

StackExchange Code Review Q#120755, answer score: 3

Revisions (0)

No revisions yet.