patterncppMinor
Bi-directional atoi()
Viewed 0 times
directionalatoistackoverflow
Problem
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 = '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 = '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";
}Solution
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.
Since you're not going to modify the original string, the parameter should be
This is an...unusual indentation style. Personally, I'd normally prefer:
Others prefer:
Given the degree to which these two styles dominate, I'd nearly always choose between them rather than trying to invent yet another.
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
You have this indented under the
I'd also prefer to use
Finally, this can fail to correctly convert at least one legitimate input. For example, given a 16-bit
With a 32-bit or 64-bit
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 = '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).Code Snippets
int atoiLefttoRight(char *s){int num = 0;
int negative = 1;
if(s)
{
if(*s == '-')if (s) {
if (*s == '-')if (s)
{
if (*s == '-'){
negative = -1;
s++;Context
StackExchange Code Review Q#55048, answer score: 4
Revisions (0)
No revisions yet.