5
\$\begingroup\$

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";
}
\$\endgroup\$
4
\$\begingroup\$

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).

| improve this answer | |
\$\endgroup\$

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.