Implementation of atoi()

I implemented the atoi() function! Here is my code:

int my_atoi(char* pointer)
{
    int result = 0;
    char* pointer1;
    multiplier = 1;
    char sign = 1;

    if(*pointer == '-')
    sign =- 1;  

    pointer1 = pointer;

    while(*pointer != '\0')
    {
        if(*pointer >= '0' && *pointer = '0' && *pointer <= '9')
        {
            result = result + ( (*pointer%48)  * multiplier);
            multiplier = multiplier / 10;       
        }

        pointer = pointer+1;
    }

    return (result * sign) / 10;
}

I wonder if there is any way that I can improve my function. I know there is a problem with my function. What if the user wants to convert from char* to int this string: "232-19". What should I do then? Any advice would be really helpful!

Things you could improve

Variables/Initialization

-
Where do you declare multiplier? I assume that since it is not declared within the method, it is declared as a global variable. Try to avoid global variables.

The problem with global variables is that since every function has access to these, it becomes increasingly hard to figure out which functions actually read and write these variables.

To understand how the application works, you pretty much have to take into account every function which modifies the global state. That can be done, but as the application grows it will get harder to the point of being virtually impossible (or at least a complete waste of time).

If you don't rely on global variables, you can pass state around between different functions as needed. That way you stand a much better chance of understanding what each function does, as you don't need to take the global state into account.

So instead of using global variables, initialize the variables in main(), and pass them as arguments to functions if necessary. In this case, I don't see the need for multiplier to be used outside of the function at all, so simply keep it declared within the function.

-
sign should be an int, and not a char.

Algorithm

-
Right now you are implementing a complicated and hard to follow method of converting a character into a number. The easy way is to have isdigit() do the hard work for you. This also will help you implement the DRY principle.

while(*pointer != '\0')
{
    if(*pointer >= '0' && *pointer = '0' && *pointer <= '9')
    {
        result = result + ( (*pointer%48)  * multiplier);
        multiplier = multiplier / 10;       
    }

    pointer = pointer+1;
}

See how you have two loops doing almost identical things? Here is how I simplified all of that by using isdigit().

while (isdigit(*c)) 
{
    value *= 10;
    value += (int) (*c - '0');
    c++;
}

You loop through the characters in the string as long as they are digits. For each one, add to the counter you're keeping - the value to add is the integer value of the character. This is done by subtracting the ASCII value of '0' from the ascii value of the digit in question.

-
Note that this code doesn't handle overflow. If you pass in "89384798719061231" (which won't fit in an int), the result is undefined. The fix is simple enough, just use a long long int to mitigate against that. We'll still have issues for extremely long numbers, but fixing that so that the function works as intended is a bit more complicated.

Documentation

-
Where did all of your comments go? A newer developer would simply gawk at some of your code.

result = result + ( (*pointer%48)  * multiplier);

Comments can really go a long way into helping other understand your code. Don't go overboard with them though, you will have to balance how much of them to put into your program.

Syntax/Styling

-
This looks like a typo.

if(*pointer == '-')
    sign =- 1;

Add a space for clarity.

if(*pointer == '-') sign = -1;

-
You should not be modifying your char* you accept as a parameter into the function. Therefore, declare the parameter as constant.

int my_atoi(const char* pointer)

-
Use more shorthand operators.

pointer++;         // same as pointer = pointer+1;
multiplier /= 10;  // same as multiplier = multiplier / 10;
multiplier *= 10;  // same as multiplier = multiplier * 10;

Final Code

#include 
#include 
#include 

long long int my_atoi(const char *c)
{
    long long int value = 0;
    int sign = 1;
    if( *c == '+' || *c == '-' )
    {
        if( *c == '-' ) sign = -1;
        c++;
    }
    while (isdigit(*c))
    {
        value *= 10;
        value += (int) (*c-'0');
        c++;
    }
    return (value * sign);
}

int main(void)
{
    assert(5 == my_atoi("5"));
    assert(-2 == my_atoi("-2"));
    assert(-1098273980709871235 == my_atoi("-1098273980709871235"));
    puts("All good."); // I reach this statement on my system
}

StackExchange Code Review Q#45755, answer score: 13