HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

K&R atoi() variation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
variationatoistackoverflow

Problem

I'm currently learning C with The C Programming Language by K&R and this is my current exercise:


Rewrite appropriate programs from earlier chapters and exercises with pointers instead of array indexing. Good possibilities include getline, atoi, itoa, reverse, strindex and getop.

This is my atoi solution with pointers:

#include 

int atoi(const char *nptr)
{
    while(isspace(*nptr++)) // skip white space
        ;

    long long val = 0;
    const char *start = nptr;

    if(*nptr == '-' || *nptr == '+')
        nptr++;

    while(*nptr >= '0' && *nptr <= '9')
        val = val*10 + *nptr++ - '0';

    return (*start == '-') ? -val : val;
}


I'm currently a little bit unhappy with the idea, that my code can't do anything against values smaller than INT_MIN and values greater than INT_MAX, but I can't come up with a solution that fits my expectations of good code.

-
I could just return INT_MIN and INT_MAX, but then the caller can't decide between the actual values and an error in form of INT_MIN/INT_MAX.

-
It is also possible to use a second argument - a pointer to an int. The function will write the value into this int and will return 1 if it was successful and 0 if it wasn't.

I want to hear your thoughts on how I should handle this special case.

Solution

You have a bug right on the first statement:

while(isspace(*nptr++)) // skip white space
    ;


I didn't vote to close your post as off-topic for broken code to make a point:
it might seem cool to do two things at once in a loop condition (condition + increment),
it's not recommended, for two reasons:

  • It's best to have one statement per line



  • A for (;;) is an exception, where it's allowed



  • The loop condition should be about the condition. Leave it at that, move the incrementing logic inside the loop body in a while loop.



The bug is an off-by-one error: when the first digit is found,
the condition is false, but the pointer will be still incremented,
moving past the first digit. As a result, a string like "123" gets parsed as 23.

The fix is simple, and follows the above recommendations about writing loops:

// skip white space
while (isspace(*nptr)) {
    nptr++;
}



I'm currently a little bit unhappy with the idea, that my code can't do anything against values smaller than INT_MIN and values greater than INT_MAX, but I can't come up with a solution that fits my expectations of good code.

Hm, the method is declared to return an int...
So it seems you don't like that int is limited within INT_MIN and INT_MAX by the language. There's just one thing to do: change the return type to something bigger, like a long!

Finally, when comparing against a variable to multiple files in the same conditional expression, often it's more readable to put the conditions in numerical increasing order, for example instead of this:

while(*nptr >= '0' && *nptr <= '9')


Write like this:

while('0' <= *nptr && *nptr <= '9')

Code Snippets

while(isspace(*nptr++)) // skip white space
    ;
// skip white space
while (isspace(*nptr)) {
    nptr++;
}
while(*nptr >= '0' && *nptr <= '9')
while('0' <= *nptr && *nptr <= '9')

Context

StackExchange Code Review Q#104969, answer score: 6

Revisions (0)

No revisions yet.