2
\$\begingroup\$

I have attempted [and made one] function to convert string to integer and catch errors in pure c

Working method

The whole working method is present in code itself

The header file

The str2num.h here:-

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

int StrNumErr = 0;

int str2numErrCheck(){
    if(StrNumErr>0){
        int ab = StrNumErr;
        StrNumErr = 0;
        return ab;
    }else{
        return 0;
    }
}

char * __restrict__ str2numErrText(int errnum){
    if(errnum==22){
        return "Alphabets or Symbols present in the String";
    }
    else if(errnum==23){
        return "Two Points in one number Error";
    }
    else if(errnum==24){
        return "Minus between two numbers Error";
    }else{
        return "";
    }
}

void invalid_argument(const char * __restrict__ _err,int errcode)
{
    StrNumErr = errcode;
}



float power(float a,int b)
//can also produce results to invalid powers [ex:- 0 to power 0 returns -1 [means invalid]]
//also can return negative powers [ex:- 2 to power -3 = 0.123]
//also calculates value for float numbers but power should be integer not **float**
{
    if(a==0 && b==0){
        return -1;
    }
    if(b==0){
        return 1;
    }
    if(b<0){
        float c=a;
        for(int i=0;i<-(b-1);i++)
        {
            c/=a;
        }
        return c;
    }
    float c=a;
    for(int i=0;i<b-1;i++)
    {
        c*=a;
    }
    return c;
}

float str2num_C(const char * __restrict__ _string)
{
    /*
    -=-=-=- Presenting the Str2Num_C Function -=-=-=-=-=-
    Advantages =>
    => Can return float numbers
    => Can return negative numbers
    => Also throws error if the string is not a number
    => Can execute Fast
    => Also, The code is pretty straightforward



        -=-=- How does this function Work -=-=-
    Basically, it takes the string and loops around all the charachters
    it checks if the charachter is '.' or '-' or among '0','1','2','3','4','5','6','7','8','9'
    if the charachter is none of the above it throws error because if it is none of above then 
    alphabet or symbol is used!

    if it finds '.' it checks if already point has been found or not
     if yes
    => it throws error two points in number
    
    if not
     it will make point=1[means point has been found] and pointplace to 1
    
    if it finds '-', it makes _minus = -1 which is multiplied at last to return minus number,
    ex- "-98" returns 98 * minus = 98 * -1 = -98
    also however if the minus is in between numbers it throws an error ex- "98-67" throws error
    
    For all numbers it finds (0-9) as num
    if it is first number, make num=number
    else
    it will multiply number by 10 and add the num

    for ex-
    98

    num = 9
    num = num * 10 + 8 [ which is 90 + 8 = 98 ]
    */
    int _strlen = strlen(_string);
    int _point=0;
    int _minus = 1;
    int _pointplace=0;
    char _curchar;
    float _return_num;
    int _numfound = 0;
    for(int _var=0;_var<_strlen;_var++)
    {
        //store current charachter
        _curchar = _string[_var];
        if(_curchar=='.'){
            //if the value contains point!
            
            if(_point==1){ //checking if already point is defined
                //two points in one number huh?>
                //error here :D
                invalid_argument("Str2Num_C Error: Two Points in one number",23);
                return 0;
            }
            //makes that _point is 1 [i.e point is true]
            //and the pointplace is 1  like in 9.86 ['8' is in 1st pointplace]
            _point=1;
            _pointplace=1;
        }
        else if(_curchar=='-'){
            //if the value contains minus
            //check if its first position like "-98"
            //if not then its something like "34-56" but that in whole is not number so throw error
            if(_var==0){
                _minus=-1;
            }else{
                invalid_argument("Str2Num_C Error: Minus in between numbers\n",24);
                return 0;
            }
        }
        else if(_curchar=='0'){
            //if char is 0!
            if(!(_numfound==1)){
                _numfound=1;
                _return_num = 0;
            }
            else if(_point==1){
                _return_num+= 0/(power(10,_pointplace));
                _pointplace+=1;
            }
            else{
                _return_num*=10;
                _return_num+=0;
            }
        }
        else if(_curchar=='1'){
            //if char is 1!
            if(!(_numfound==1)){
                _numfound=1;
                _return_num = 1;
            }
            else if(_point==1){
                _return_num+= 1/(power(10,_pointplace));
                _pointplace+=1;
            }
            else{
                _return_num*=10;
                _return_num+=1;
            }
        }
        else if(_curchar=='2'){
            //if char is 2!
            if(!(_numfound==1)){
                _numfound=1;
                _return_num = 2;
            }
            else if(_point==1){
                _return_num+= 2/(power(10,_pointplace));
                _pointplace+=1;
            }
            else{
                _return_num*=10;
                _return_num+=2;
            }
        }
        else if(_curchar=='3'){
            //if char is 3!
            if(!(_numfound==1)){
                _numfound=1;
                _return_num = 3;
            }
            else if(_point==1){
                _return_num+= 3/(power(10,_pointplace));
                _pointplace+=1;
            }
            else{
                _return_num*=10;
                _return_num+=3;
            }
        }
        else if(_curchar=='4'){
            //if char is 4!
            if(!(_numfound==1)){
                _numfound=1;
                _return_num = 4;
            }
            else if(_point==1){
                _return_num+= 4/(power(10,_pointplace));
                _pointplace+=1;
            }
            else{
                _return_num*=10;
                _return_num+=4;
            }
        }
        else if(_curchar=='5'){
            //if char is 5!
            if(!(_numfound==1)){
                _numfound=1;
                _return_num = 5;
            }
            else if(_point==1){
                _return_num+= 5/(power(10,_pointplace));
                _pointplace+=1;
            }
            else{
                _return_num*=10;
                _return_num+=5;
            }
        }
        else if(_curchar=='6'){
            //if char is 6!
            if(!(_numfound==1)){
                _numfound=1;
                _return_num = 6;
            }
            else if(_point==1){
                _return_num+= 6/(power(10,_pointplace));
                _pointplace+=1;
            }
            else{
                _return_num*=10;
                _return_num+=6;
            }
        }
        else if(_curchar=='7'){
            //if char is 7!
            if(!(_numfound==1)){
                _numfound=1;
                _return_num = 7;
            }
            else if(_point==1){
                _return_num+= 7/(power(10,_pointplace));
                _pointplace+=1;
            }
            else{
                _return_num*=10;
                _return_num+=7;
            }
        }
        else if(_curchar=='8'){
            //if char is 8!
            if(!(_numfound==1)){
                _numfound=1;
                _return_num = 8;
            }
            else if(_point==1){
                _return_num+= 8/(power(10,_pointplace));
                _pointplace+=1;
            }
            else{
                _return_num*=10;
                _return_num+=8;
            }
        }
        else if(_curchar=='9'){
            //if char is 9!
            if(!(_numfound==1)){
                _numfound=1;
                _return_num = 9;
            }
            else if(_point==1){
                _return_num+= 9/(power(10,_pointplace));
                _pointplace+=1;
            }
            else{
                _return_num*=10;
                _return_num+=9;
            }
        }else{
            invalid_argument("The string contains alphabetic charachters or symbol",22);
            return 0;
        }
        
        
    }
    return _return_num*_minus;
}

Example usage:

#include<stdio.h>
#include<string.h>
#include<stdlib.h>
#include "str2num.h"



int main()
{
    //declaring variables
    char string[20];
    float num;
    int err;
    
    //input a string
    printf("Enter a number: ");
    gets(string);
    
    //convert into number and add 2
    num = str2num_C(string);
    num+=2;
    
    //check errors
    err = str2numErrCheck();
    if(err>0){
        printf("ERROR: %s",str2numErrText(err));
    }
    else{
        printf("Number + 2 is: %f",num);
    }
    
}

EDIT: the invalid_argument() also has a field to give string to it because when converting this code to c++ you just have to add throw and remove the numbers

\$\endgroup\$
2
  • \$\begingroup\$ Hmmm, "Convert string to integer and catch errors" and test code uses gets()??? \$\endgroup\$ Oct 14 at 17:02
  • \$\begingroup\$ @chux i didnt know that - i am new to c-programming as i am :l \$\endgroup\$
    – Pear
    Oct 15 at 3:01
3
\$\begingroup\$

Repeated code

You do the following once for every possible digit:

if(_curchar=='x'){
    //if char is 0!
    if(!(_numfound==1)){
        _numfound=1;
        _return_num = x;
    }
    else if(_point==1){
        _return_num+= x/(power(10,_pointplace));
        _pointplace+=1;
    }
    else{
        _return_num*=10;
        _return_num+=x;
    }
}

Unless I overlooked something, this could very well be replaced with a general approach:

// get int val of character. this is a quick-and-dirty way and may not work with non-ascii strings!
int char_val = _curchar - '0';

if(!(_numfound==1)){
    _numfound=1;
    _return_num = char_val; 
}
else if(_point==1){
    _return_num+= char_val/(power(10,_pointplace));
    _pointplace+=1;
}
else{
    _return_num*=10;
    _return_num+=char_val;
}

Now you won't have to do a bunch of else ifs as this works for every valid numeric digit. For the latter, this may be checked with a condition between the digit conversion and the check for . and -:

if (_curchar < '0' || _curchar > '9') {
    invalid_argument("The string contains alphabetic charachters or symbol",22);
}

invalid_argument

Passing a string you never use a function is pointless, and only setting a global error code could very well be done directly inside the function.

Error codes

Magic number alert! Your error codes should either be a const int or be #defined. It's easier to see what happens if set your error var to, say ERR_INV_CHAR instead of 22. This makes things easier to chage as well. The reason that functions like strerror or str2numErrText exist is a similar reason: the user doesn't have to look up "error code 22" but instead can just str2numErrText(22) and get something useful.

C booleans

You use some vars as a boolean flag (e.g. _point). They should be renamed to something like _point_found to express this usage. Secondly, C evaluates 0 as false and 1 (or every other int for that matter) as true. This means that you can do the follwing:

if (_point_found == 1)  // feels like writing "if _point_found == True" in python. 
if (_point_found)       // good

And if you want to go the distance, import <stdbool.h> and do this:

int _point_found = 1;      // "classic" way
bool _point_found = true;  // with stdbool.h

Correct use of headers

As @aghast pointed out, this is not how you use headers. I'll provide an example to illustrate further:

// str2num.h
////////////////////

// include guard to stop multiple inclusions from different files.
#ifndef STR2NUN_INCL
#define STR2NUN_INCL

const int ERR_INV_CHAR;

int str2numErrCheck();
void invalid_argument(const char * __restrict__ _err,int errcode);

#endif
// str2num.c
////////////////////

#include "str2num.h"

const int ERR_INV_CHAR = 22;

int StrNumErr = 0;

int str2numErrCheck(){
    if(StrNumErr>0){
        int ab = StrNumErr;
        StrNumErr = 0;
        return ab;
    }else{
        return 0;
    }
}

void invalid_argument(const char * __restrict__ _err,int errcode)
{
    StrNumErr = errcode;
}

You now keep the implementation in str2num.c and include str2num.h as you've done before.

\$\endgroup\$
10
  • \$\begingroup\$ I am new to #ifdef and #ifndef stuff! this is perfect now i am understanding about it was expecting yours like answer. I know @aghast is also correct but i didnt understood what he was trying to say [ I am new to c so..... ] You made me crystal clear thanx! \$\endgroup\$
    – Pear
    Oct 14 at 12:20
  • \$\begingroup\$ would you mind if you tell me how do i make the header file such a way that in main() program i can use functions and i dont have to create it into another file. If u dont understand what i am trying to say:- 1) i want the header to be used in different files , 2) i dont want to repeteadly create invalid_argument() and strErr functions, 3) i want the functions to be working inside any file with minimal code and all the stuff should be stored inside other things probably header files? But i have to say Repetition is gone thx and i also work with python how did u guess? :O \$\endgroup\$
    – Pear
    Oct 14 at 12:27
  • \$\begingroup\$ Magic number alert! that really got me i could've done that, my mind :| \$\endgroup\$
    – Pear
    Oct 14 at 12:43
  • \$\begingroup\$ 1) You can. Whereever a method from str2num.c is needed, include str2num.h and it'll work. 2) You don't have to...? Just the function declaration in the .h and the function implementation in the .c file. That's one line you'll duplicate. 3) Again, they should work everywhere. What do you mean with "stuff"? \$\endgroup\$ Oct 14 at 12:57
  • \$\begingroup\$ Also I used python for no reason in that example, but it's the only language I know that uses underscores for internals. \$\endgroup\$ Oct 14 at 12:58
3
\$\begingroup\$

There are a few things about your code that can be improved.

Style

You obviously have a style, which is to the good. But it's one I have some problems with:

  1. Spaces. I don't know why you are so chary with your horizontal space, but I suggest you amend your style to favor more white space rather than less. Many operations, both manual and automated, benefit from having spaces separating tokens. For example, "filling" text (to break code at a max. width boundary), transposing words (changing if for else), searching for and/or replacing identifiers all are technically possible without spaces, but are much easier when spaces are present.

  2. Underscores. Leading underscores are not universally reserved by the standard. But entire classes of them are reserved, so your habit suffers from (a) making readers uncomfortable, and (b) being harder to type. I don't know of any keyboard where the underscore is easy to type by default. (I have modified my own editor to support SHIFT+SPACE as underscore. But this is still a shifted key.) On a US keyboard, a leading underscore by default will displace one or both hands just as you begin typing the name. I suggest you adopt a convention that lets you create short, simple, and fast names, especially for temporaries and loop variables.

  3. Organization. Header files are for declarations, types, constants, and inline definitions that cannot be placed elsewhere. You are including your entire source code in the header, which is likely to cause problems in the future if more than one file includes the header and creates parallel definitions of the same functions in different object files. I suggest you partition your code so that declarations remain in the header while function definitions are in a separate .c file.

Implementation

There are some problems with your implementation, as well:

  1. Your code returns a float. That is the smallest of the standard floating point values, which means you cannot usefully parse wider floating point values from input. I believe it would be better to return a wider value and then narrow it down into a smaller storage unit if need be. Return a double or something wider, then let the user convert it down to float if they wish.

  2. Your use of the __restrict__ keyword is inappropriate. First, because it is an implementation extension -- the correct keyword is restrict. And second because applying it to the return value of a function makes little sense. I could easily call your function twice and violate the restrict contract, which would lead to undefined behavior.

  3. You should mark string literals as const char * unless you have a compelling reason not to. (Which you do not!)

  4. Floating point values already have a built-in "bogus" value. Using 0 as some kind of "null float" is nonsensical. Instead, use NAN from math.h and let the user check with isnan().

  5. You are accumulating floating point errors all over the place. Every time you perform a floating point operation, you introduce a source of error. Every addition, every division, is a place where one of many discrepancies can find their way into your results. I suggest you rely more on the standard functions, like pow() (or powf() for floats) and less on hand-rolled code. Also, consider parsing your values as integers and only converting them to floats once you have finished parsing the digits successfully. (Something like "12.34" -> whole_part=12, frac_part=34, frac_div=100, then when finished do result = whole_part + (float)frac_part / frac_div)

\$\endgroup\$
6
  • \$\begingroup\$ The reason behind my horizontal space is because i focus to write code then making them stylish i hope that makes sense and saves time?. anyways your suggestion is great! i am newbie in here as well as in c programming. i looked up some tutorials and started coding so obviously there won't be good codes :) but thanx for a reveiw! i use underscores for function variables so i dont get confused them with the global variables! \$\endgroup\$
    – Pear
    Oct 14 at 4:31
  • \$\begingroup\$ Hey @aghast can u make point 2 clear in implementation where exactly [ or which function ] can u call to make the reult undesired? \$\endgroup\$
    – Pear
    Oct 14 at 4:41
  • \$\begingroup\$ if i was to enter only restrict it would generate error please elaborate your point 2 in implementation \$\endgroup\$
    – Pear
    Oct 14 at 8:26
  • \$\begingroup\$ @pear The restrict keyword is a contract between the code author (you) and the compiler. It guarantees that if a restricted pointer references an object, the only references to that object will be via the restricted pointer -- there are no aliases to the object through other pointers. If I call a = str2numErrText(22); and also call b = str2numErrText(22); then I have violated the restrict contract and have two unrelated pointers to the same data. As soon as I modify through one of the pointers (which you don't want me to do, but the result is not const), there is UB. \$\endgroup\$
    – aghast
    Oct 14 at 13:12
  • 1
    \$\begingroup\$ @pear It sounds like you are using gcc (since I think __restrict__ is a gcc-ism) and maybe you are using it in C89 mode? Since restrict is a C99 keyword, it should be valid if you use any modern standard. I'll suggest you use the latest standard you can get away with. If you have to use C89, then use #ifdef / #define / #endif to write a restrict macro that expands to __restrict__. (Better still, don't use restrict at all if it's not useful.) \$\endgroup\$
    – aghast
    Oct 14 at 13:15
0
\$\begingroup\$

Firstly, this code doesn't do what is described. It converts a string to float, not integer.


Your header file should contain only the public declarations. Put the implementation into a normal source file.


int StrNumErr = 0;

Avoid global variables - they make it hard to reason about code, and can inhibit concurrent use of the functions.


int str2numErrCheck(){

Should be declared as taking no arguments, i.e. int str2numErrCheck(void). And we're not allowed to give it a name beginning with str - it needs to be changed to one that's not reserved for <string.h>.

    if(StrNumErr>0){
        int ab = StrNumErr;
        StrNumErr = 0;
        return ab;
    }else{
        return 0;
    }
}

As we never set StrNumErr to a negative value, and both branches have the same observable behaviour for StrNumErr==0, then we don't need the test. We could do with a better variable name than ab, too.


char * __restrict__ str2numErrText(int errnum){

The __restrict__ doesn't provide any value here.

if(errnum==22){
    return "Alphabets or Symbols present in the String";
}

What's the significance of 22? Why aren't we using named (enum) constants?

We shouldn't be returning const char* from a function declared to return char *. I suggest changing the function to return const char* instead.


void invalid_argument(const char * __restrict__ _err,int errcode)
{
    StrNumErr = errcode;
}

Again, no benefit from __restrict__ here. Avoid declaring identifiers beginning with _ (since that's easier than remembering exactly which ones are safe, and where) - and why are we ignoring _err anyway? That leads to inconsistency between what the caller provides and what we return from str2numErrText().


float power(float a,int b)

This function should be declared with static linkage, unless you really intend it to be part of the public interface.

The usual floating-point type is double. Calculating with float sacrifices a lot of precision.

And calculating powers by repeated multiplication is inefficient compared to binary exponentiation.

I don't see any benefit to using this function rather than the standard pow() from <math.h>.


    int _strlen = strlen(_string);

Use the right type - strlen() returns a size_t. But we don't need to measure the string anyway, as we're walking it character by character - we should loop like a native with for (const char *s = string; *s; ++) (or just use the passed string pointer here. Code that does unnecessary string indexing looks like it's written by a FORTRAN or Java programmer who is still uncomfortable with pointers.


float str2num_C(const char * __restrict__ _string)

Another reserved identifier, and another pointless __restrict__.


    else if(_curchar=='0'){
        //if char is 0!
        if(!(_numfound==1)){
            _numfound=1;
            _return_num = 0;
        }
        else if(_point==1){
            _return_num+= 0/(power(10,_pointplace));
            _pointplace+=1;
        }
        else{
            _return_num*=10;
            _return_num+=0;
        }
    }
    else if(_curchar=='1'){
        //if char is 1!
        if(!(_numfound==1)){
            _numfound=1;
            _return_num = 1;
        }
        else if(_point==1){
            _return_num+= 1/(power(10,_pointplace));
            _pointplace+=1;
        }
        else{
            _return_num*=10;
            _return_num+=1;
        }
    }

These branches can all be combined, using the fact that C guarantees that the encoding of digits 0...9 are contiguous (so curchar - '0' gives the integer value of the digit).

Instead of computing power() each time around, it might be worth keeping that value in a variable, and scaling it by 10 each time it's used:

double place_value = 1;
⋮

for (…) {
    ⋮
    if (curchar >= '0' && curchar <= '9') {
        int digit = curchar - '0';
        if (!numfound) { numfound = 1; }
        if (point) {
            place_value /= 10;
        }
        return_num = return_num * 10 + digit;
    }
    ⋮
}
return return_num * minus * place_value;

            invalid_argument("The string contains alphabetic charachters or symbol",22);

Check your spelling (characters). We might have spotted this if we'd actually used the value.


    //declaring variables
    char string[20];
    gets(string);

That's probably the most useless comment I've seen today.

What if 20 characters or more are entered? Eschew gets() - that's the most dangerous function in the Standard Library (it's impossible to use it safely). Use fgets():

fgets(string, sizeof string, stdin);

And don't forget to examine its return value before relying on string containing anything valid.

    num = str2num_C(string);

Well, that's not going to work, because str2num_C() will error on the newline character that gets() read.


if(err>0){
    printf("ERROR: %s",str2numErrText(err));
}
else{
    printf("Number + 2 is: %f",num);

Please write full lines of output (ending in \n). It's very bad behaviour to terminate with a partial line written.

And error messages ought to be printed to stderr, not stdout.


My overall assessment is that this reinvention of the wheel is pointless anyway. It's much simpler to wrap standard parsers such as strtof() to fit the interface you need than to write this stuff from scratch. It's likely to be more efficient, too - although you'll want to run some actual benchmarks to determine that.

\$\endgroup\$
2
  • \$\begingroup\$ Thats probably the most useless comment I've seen . are newcomers not allowed here? i am a newcomer and writing that in comment i think would not change any output because from what i've learnt comments dont compile \$\endgroup\$
    – Pear
    Oct 16 at 7:25
  • \$\begingroup\$ If the comment were not there, would you struggle to understand that variables are being declared? No matter how new you are, that's pretty unlikely. So all the comment does is get in the way of the important stuff. That's the kind of comment even a beginner needs to learn isn't helpful. \$\endgroup\$ Oct 17 at 9:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.