Bi-directional atoi()

Question

I am implementing atoi() by traversing a string from the beginning as well as from the end. Is there anything I can improve?

using namespace std;


int atoiLefttoRight(char *s){
    int num = 0;
    int negative = 1;
    if(s)
        {
        if(*s == '-')
            {
            negative = -1;
            s++;
            }
            while(*s && (*s <= '9') && (*s >= '0'))
            {
            num = (num * 10) + (*s - '0');
            s++;
            }
        }
return (negative * num);
}


int atoiRighttoLeft(char *s){
    int i = 1;
    int num = 0;
    int negative = 1;

    if(s){
        if(*s == '-')
                      {
            negative = -1;
            s++;
           }
    char *temp = s;
    while(*temp)
    temp++;
    temp--;

    while(*temp && temp >= s)
    {
        if( *temp <= '9' && *temp >= '0'){
        num = num + ( (*temp - '0') * i);
        i = i*10;
        temp--;
        }
        else
        {
        temp--;
        num = 0;
        i = 1;
        }
    }
    }
return (negative * num);
}

int main()
{
cout<<atoiLefttoRight("-12ab3")<<"\n";
cout<<atoiRighttoLeft("1234h5")<<"\n";
}

Answer 1

For the moment I'll concentrate on the left to right version. Most of the same comments apply to the right to left version as well though.

int atoiLefttoRight(char *s){

Since you're not going to modify the original string, the parameter should be char const *s.

    int num = 0;
    int negative = 1;
    if(s)
        {
        if(*s == '-')

This is an...unusual indentation style. Personally, I'd normally prefer:

if (s) { 
    if (*s == '-')

Others prefer:

if (s)
{
    if (*s == '-')

Given the degree to which these two styles dominate, I'd nearly always choose between them rather than trying to invent yet another.

            {
            negative = -1;
            s++;

Although it's unlikely (in the extreme) to make a difference in this case, in cases like this where you can use either prefix or postfix increment (or decrement) I'd choose the prefix form, making this ++s;

            }
            while(*s && (*s <= '9') && (*s >= '0'))

You have this indented under the if statement as if it were controlled by the if statement (but it's not).

I'd also prefer to use isdigit to determine whether a character is a digit.

Finally, this can fail to correctly convert at least one legitimate input. For example, given a 16-bit int, an input of -32768 is legitimate meaningful--but this won't convert it correctly, because it tries to represent the digits as a positive signed number (but the largest signed 16-bit number is 32767).

With a 32-bit or 64-bit int, the number gets larger but the same basic idea applies (and if you had an even larger int, such as 128-bit, the same would still be true--bigger number, but still a number where it would fail).