Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is this a bad way to check if a string represents a floating point number?

Tags:

Looking around online I didn't find a solution that satisfied me, so I tried it myself. But now in a lecture it was also said that calling functions and waiting for returns might cause the stack to overflow, so is this a bad idea? I use this function to check whether argv[1] is a float or not. Would a loop be better? or is there something way more intuitive? there must be exactly one point and it has to be followed by atleast one digit right?

#include <stdbool.h>
#include <ctype.h>

 /**
 * checks if string is floating point number
 * please call function with pointCounter=0 and digitAfterPoint=false
 */
bool isFloatString(char *s, int pointCounter, bool digitAfterPoint) 
{                                                                   

    if (isdigit(*s))
    {
        if(pointCounter==1)
        {
            digitAfterPoint=true;
        }
        return  isFloatString(s+1, pointCounter, digitAfterPoint);
    }

    else if (*s == '.' && pointCounter==0)
    {
        return isFloatString(s+1, pointCounter+1,digitAfterPoint);
    }
    else if (*s == '\0' && digitAfterPoint)
    {
        return true;
    }
    else
    {
        return false;
    }

}
like image 781
Duc Nguyen Avatar asked Dec 20 '17 06:12

Duc Nguyen


1 Answers

For 999 digits and one point, there are 1000 recursive calls with each return address, and three parameters on the stack. I would find that okay. However a non-recursive iterative solution does away with the state parameters, and is easier to read (in this case only).

bool isFloatString(char *s)
{
    int pointCounter = 0;
    bool digitAfterPoint = false;
    while (*s != '\0')
    {
        if (isdigit(*s))
            digitAfterPoint = pointCounter == 1;
        }
        else if (*s == '.' && pointCounter == 0)
        {
            ++pointCounter;
        }
        else
        {
            return false;
        }
        ++s;
    }
    return digitAfterPoint;
}

Mind: the recursive solution is subject to a malicious stack overflow.


@MatteoItalia rightly indicated that there is only tail recursion (nothing is done with the result), so any mature C/C++ compiler would transform the recursion to jumps (iteration). Here his disassembly (see link in comment too).

isFloatString(char*, int, bool):
  movsx ecx, BYTE PTR [rdi]
  mov r9d, edx
  mov r8d, ecx
  sub ecx, 48
  cmp ecx, 9
  jbe .L23
  cmp r8b, 46
  je .L24
  test r8b, r8b
  sete al
  and eax, edx
  ret
.L24:
  xor eax, eax
  test esi, esi
  je .L25
.L1:
  rep ret
.L23:
  movsx eax, BYTE PTR [rdi+1]
  mov ecx, eax
  sub eax, 48
  cmp esi, 1
  je .L26
  cmp eax, 9
  movzx edx, dl
  jbe .L10
  cmp cl, 46
  je .L27
.L8:
  test cl, cl
  sete al
  and eax, r9d
  ret
.L26:
  cmp eax, 9
  jbe .L28
  xor eax, eax
  cmp cl, 46
  mov r9d, 1
  jne .L8
  jmp .L1
.L28:
  mov edx, 1
.L10:
  add rdi, 2
  jmp isFloatString(char*, int, bool)
.L25:
  movzx edx, dl
  add rdi, 1
  mov esi, 1
  jmp isFloatString(char*, int, bool)
.L27:
  xor eax, eax
  test esi, esi
  jne .L1
  add rdi, 2
  mov esi, 1
  jmp isFloatString(char*, int, bool)
like image 187
Joop Eggen Avatar answered Sep 19 '22 13:09

Joop Eggen