Cosas que podrías mejorar

Variables / Inicialización

• ¿Dónde declaras multiplier? Supongo que, dado que no se declara dentro del método, se declara como una variable global. Trate de evitar las variables globales.

  El problema con las variables globales es que dado que cada función tiene acceso a estas, se vuelve cada vez más difícil averiguar qué funciones realmente leen y escriben estas variables.

  Para comprender cómo funciona la aplicación, es necesario tener en cuenta todas las funciones que modifican el estado global. Eso se puede hacer, pero a medida que la aplicación crezca se volverá más difícil hasta el punto de ser virtualmente imposible (o al menos una completa pérdida de tiempo).

  Si no confía en variables globales, puede pasar el estado entre las diferentes funciones según sea necesario. De esa manera, tendrá muchas más posibilidades de comprender lo que hace cada función, ya que no es necesario tener en cuenta el estado global.

  Entonces, en lugar de usar variables globales, inicializa las variables en main() y pásalas como argumentos a funciones si es necesario. En este caso, no veo la necesidad de que multiplier se use fuera de la función en absoluto, así que simplemente manténgalo declarado dentro de la función.

• sign debe ser un int, y no un char .

Algoritmo

• Ahora mismo está implementando un método complicado y difícil de seguir para convertir un carácter en un número. La forma más fácil es hacer que isdigit() haga el trabajo duro por usted. Esto también le ayudará a implementar el principio SECO .

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

  ¿Ves cómo tienes dos bucles que hacen cosas casi idénticas? Así es como simplifiqué todo eso usando isdigit().

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

  Recorre los caracteres de la cadena siempre que sean dígitos. Para cada uno, agregue al contador que está guardando – el valor a agregar es el valor entero del carácter. Esto se hace restando el valor ASCII de "0" del valor ascii del dígito en cuestión.

• Tenga en cuenta que este código no «No manejar el desbordamiento. Si pasa» 89384798719061231 «(que» no cabe en un int), el resultado no está definido. La solución es bastante simple, solo use un long long int para mitigar eso. Seguiremos teniendo problemas con números extremadamente largos, pero solucionarlo para que la función funcione como se espera es un poco más complicado.

Documentación

• ¿A dónde fueron todos sus comentarios? Un desarrollador más nuevo simplemente se quedaría boquiabierto con parte de su código.

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

  Los comentarios pueden ser de gran ayuda para ayudar a otros a comprender su código. Sin embargo, no se exceda con ellos, tendrá que sopesar la cantidad de para ponerlos en su programa.

Sintaxis / Estilo

• Esto parece un error tipográfico.

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

  Agregue un espacio para mayor claridad.

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

• Debes no modificará su char* que acepta como parámetro en la función. Por lo tanto, declare el parámetro como constante.

  int my_atoi(const char* pointer) 

• Utilice más operadores abreviados.

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

Código final

#include <stdio.h> #include <assert.h> #include <ctype.h> 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 } 

Comentarios

• No debe ‘ t cambiar los tipos de retorno arbitrariamente. atoi() tradicionalmente devuelve un int, por lo que my_atoi() también debería hacerlo. Si desea analizar un long long, emule strtoll().
• isdigit(*c) no está definido para *c valores menores que 0 (distintos de EOF). Es mejor while (isdigit((unsigned char) (*c) ))
• Esquina perdida: cuando my_atoi() el resultado debe ser LLONG_MIN, value += (int) (*c-'0'); es un desbordamiento de enteros con signo (UB) cuando intenta formar LLONG_MAX + 1.
• Usando isdigit es incorrecto en absoluto, ya que no ‘ t tiene una función relacionada numeric_value. Por lo tanto, si su juego de caracteres tiene dos rangos de dígitos (0 a 9 y ٠ a ٩), los números índicos se analizarán incorrectamente. Solo manténgase en '0' <= c && c <= '9' para estar seguro. Esto también evita que el comportamiento indefinido utilice incorrectamente la función ctype.
• Se perdió un punto importante cuando escribió » valor ASCII de ‘ 0 ‘ » : hay ‘ s nada que diga que el conjunto de caracteres del host debe ser ASCII (solo que 0..9 son contiguos). Esa ‘ es la razón por la que escribe '0' en lugar de un número de punto de código específico de la codificación.

[Editar]

Excepto por el comportamiento en caso de error, atoi() es equivalente a (int)strtol(nptr, (char **)NULL, 10). strtol() acepta espacios en blanco iniciales. OP «s my_atoi(char* pointer) no. Para remediar:

int my_atoi(const char* pointer) { while (isspace((unsigned char) *pointer)) { pointer++; } ... 

A continuación se describe una buena forma de manejar INT_MIN.

OTOH, la entrega de valores fuera de [INT_MIN...INT_MAX] no está definida por la especificación C, por lo que algunas simplificaciones pueden ser tenía. Consulte más abajo.

Cuando una cadena representa INT_MIN, (supongamos «s de 32 bits int) como "-2147483648", el código se ejecuta en int desbordamiento intentando calcular 2147483648. Una forma sencilla de resolver esto es, en lugar de encontrar el valor positivo y luego negarlo, adoptar el lado negativo de las cosas. Al hacer la mayor parte de las matemáticas en el rango de INT_MIN a 0, evitamos UB. Lado negativo: algunos encuentran este enfoque más difícil de seguir.

Ir a un número entero más amplio o unsigned no siempre es posible ya que el tamaño entero de «texto- -> la rutina integer «puede tener el tamaño máximo. Estrictamente hablando, unsigned no siempre tiene un rango positivo más amplio que int. En cualquier caso, todas las matemáticas se pueden manejar con el tamaño de entero con signo deseado sin recurrir a otros tipos.

#include <ctype.h> #include <limits.h> int my_atoi(const char* pointer) { // good idea to make the `const` int result = 0; while (isspace((unsigned char) *pointer)) { pointer++; } char sign = *pointer; if (*pointer == "-" || *pointer == "+") { // text could lead with a "+" pointer++; } int ch; // isdigit() expects an unsigned char or EOF, not char while ((ch = (unsigned char)(*pointer)) != 0) { if (!isdigit(ch)) break; ch -= "0"; // Will overflow occur? if ((result < INT_MIN/10) || (result == INT_MIN/10 && ch > -(INT_MIN%10))) Handle_Overflow(); result *= 10; result -= ch; // - , not + pointer++; } if (sign != "-") { if (result < -INT_MAX) Handle_Overflow(); result = -result; } return result; } 

Notas:

pointer%48 es confuso. ¿Qué tiene de especial 48? Si te refieres a "0", usa pointer % "0".

«string:» 232-19 «. ¿Qué debo hacer entonces? » Recomendamos detener la conversión en «232» y devolver el valor 232. Podría establecer errno, pero el típico atoi() la función no maneja demasiado los errores.

En caso de desbordamiento, la configuración de errno podría suceder, pero nuevamente, el típico atoi() no maneja demasiado los errores. Sugiera una devolución simple de INT_MAX o INT_MIN.

Si desea un mejor manejo de errores, cambie a algo como lo siguiente y establecer un estado de error.

int my_atoi(const char *s, int *ErrorCode); 

o ubicación donde terminaron las cosas. Si esto es bueno, terminaron en el "\0".

int my_atoi(const char *s, const char **endptr); 

[Editar] Simplificado: eliminado detección fuera de rango ya que la especificación C lo permite. «Si el valor del resultado no se puede representar, el comportamiento no está definido.

int my_atoi(const char* pointer) { int result = 0; while (isspace((unsigned char) *pointer)) { pointer++; } char sign = *pointer; if (*pointer == "-" || *pointer == "+") { pointer++; } while (isdigit((unsigned char)*pointer)) { result = result*10 - (*pointer++ - "0"); } if (sign != "-") { result = -result; } return result; } 

Comentarios

• INT_MIN/10 y INT_MIN%10 requieren un comportamiento C99.

 char sign = *pointer; if (*pointer == "-" || *pointer == "+") { pointer++; } 

Por qué eliminar «puntero» ¿tres veces? Una vez es suficiente:

 char sign = *pointer; if (sign == "-" || sign == "+") { pointer++; } 

Comentarios

• Bienvenido a Code Review, tu primera respuesta parece buena , ¡Disfruta tu estancia! Aunque me pregunto si hace una diferencia en el código generado.

si está de acuerdo con la recursividad entonces el código podría acortarse a uno debajo

#include <string.h> #include <math.h> #include <stdbool.h> int natural_number(const char* string) { int index = strlen(string) - 1; int number = pow(10, index) * (*string - "0"); return (index == 0) ? number : number + natural_number(string + 1); } int my_atoi(const char* string) { int sign = (*string == "-") ? -1 : 1; int offset = (*string == "-") ? 1 : 0; return sign * natural_number(string + offset); } /* test cases */ my_atoi("-100") == -100; my_atoi("0") == 0; my_atoi("100") == 100; 

El agotamiento de la pila podría mitigarse con la -foptimize-sibling-calls bandera del compilador, que es compatible con compiladores de GCC y Clang.

Actualización:

Como se indicó por la implementación de Roland Illig no maneja la entrada con formato incorrecto. Si lo desea, siga la atoi semántica , entonces el siguiente código debe ser bien _{no olvide configurar Compile Options en uno en los comentarios} .

int digit(char symbol) { return symbol - "0"; } /* tail call optimized */ int natural_number_tc(const char* string, int number) { return !isdigit(*string) ? number : natural_number_tc(string + 1, 10 * number + digit(*string)); } int natural_number(const char* string) { return natural_number_tc(string, 0); } const char* left_trim_tc(const char* string, const char* symbol) { return !isspace(*string) ? symbol : left_trim_tc(string + 1, symbol + 1); } const char* left_trim(const char* string) { return left_trim_tc(string, string); } int my_atoi(const char* string) { const char* symbol = left_trim(string); int sign = (*symbol == "-") ? -1 : 1; size_t offset = (*symbol == "-" || *symbol == "+") ? 1 : 0; return sign * natural_number(symbol + offset); } 

Este sigue siendo el código de chux donde los bucles se reemplazan con recursividad

int result = 0; while (isdigit((unsigned char)*pointer)) { result = 10 * result + (*pointer - "0"); pointer++; } // VS int loop(const char* pointer, int result) { return !isdigit((unsigned char)*pointer) ? result : loop(pointer + 1, 10 * result + (*pointer - "0")) } 

Comentarios

• Caso de prueba: buf = malloc(65536); buf[0] = '\0'; my_atoi(buf) probablemente fallará.
• Caso de prueba: bufsize = 1 << 20; buf = malloc(bufsize); memset(buf, '0', bufsize); buf[bufsize - 1] = '\0'; my_atoi(buf) tomará mucho mucho tiempo.

Para un ejercicio en leetcode , escribió lo siguiente impl: código cpp atoi

 class Solution { private: bool checkMin(int a, int b=10, int c=0, int min_val=INT_MIN) { /* accepts a*b + c, min a>min; b>min; c>min check a*b+c > min or not b>0; a<0 -ive; c<0 a!=0 */ min_val = min_val -c; //std::cout<<"new min input: "<<a <<" , "<< c<<" iter: "<<b << " "<<min_val <<std::endl; //compare with a now if(a<min_val) return false; int cur_prod = 0; if(a==0) return true; for(;b>1;b--) { cur_prod += a; int curr_diff = min_val-cur_prod; /* subtraction possible because min_val<prod, min_val-prod<prod-prod min_val-prod<0 ---1 prod<0 -prod>0 min_val+(-prod )> min_val+0 [x+ (+ive quantity)>x ] min_val-prod>min_val --2 from 1, 2 min_val< min_val-prod < 0 ---3 from 3, min_val-prod can be expressed in integer check if curr_diff still can hold a deduction of a which means: curr_diff<a should hold, for a further a deduction in prod -5, -6 for ex of min_val = 59, a = -6 at b = 2 (9th iteration) prod = -54 you can"t add -6 now, since it will cross definable limit only b-1 iterations because at i-1 th iteration, ith product formation is checked */ //std::cout<<"check function for input: "<<a <<" , "<< c<<" iter: "<<b << " prod now = " //<< cur_prod << " diff = " <<curr_diff<<" is curr_dif<a "<<(curr_diff<a)<<std::endl; if(curr_diff>a) { //std::cout<<" not possible"<<std::endl; return false; } } return true; } bool checkMax(int a, int b=10, int c=0, int max_val=INT_MAX) { /* accepts a*b + c, min a<max; b<max; c<max check a*b+c < max or not b>0; a>0, c>0 */ max_val = max_val -c; //std::cout<<"new max input: "<<a <<" , "<< c<<" iter: "<<b << " "<<max_val <<std::endl; //compare with a now if(a>max_val) return false; int cur_prod = 0; if(a==0) return true; for(;b>1;b--) { cur_prod += a; int curr_diff = max_val-cur_prod; /* subtraction possible because max_val>prod, max_val-prod>prod-prod max_val-prod>0 ---1 prod>0 -prod<0 max_val+(-prod )< max_val+0 [x+ (-ive quantity)<x ] max_val-prod<max_val --2 from 1, 2 0< max_val-prod < max_val ---3 from 3, max_val-prod can be expressed in integer check if curr_diff still can hold a increment of a which means: curr_diff>a should hold, for a further a deduction in prod 5>6 fails for ex of max_val = 59, a = 6 at b = 2 (9th iteration) prod = 54 you can"t add 6 now, since it will cross definable limit only b-1 iterations because at i-1 th iteration, ith product formation is checked */ //std::cout<<"check function for input: "<<a <<" , "<< c<<" iter: "<<b << " prod now = " // << cur_prod << " diff = " <<curr_diff<<" is curr_dif<a "<<(curr_diff>a)<<std::endl; if(curr_diff<a) { //std::cout<<" not possible"<<std::endl; return false; } } return true; } public: int myAtoi(string str) { //code to trim string int i =0, end=str.length()-1; //std::cout<<i<<" "<<end<<std::endl; while(i<end && str.at(i)==" ") {i++;continue;} while(end>-1 && str.at(end)==" ") {end--;continue;} if(end<i) return 0; int sign=1; if(str.at(i)=="-") {sign = -1; i++;} else if(str.at(i)=="+") {i++;} string tr_str = str.substr(i, end-i+1); int num = 0; for(char& digit : tr_str) { if(digit<"0" || digit>"9") return num; // not convertable character - exit int c= digit-"0"; if(sign==-1) { //std::cout<<"Evaluating "<<c<<std::endl; //number cannot be lower than INT_MIN // do a check of num * 10 - c //num<0 already if(checkMin(num, 10, -c, INT_MIN)) num = num*10 -c; else { num = INT_MIN; break; } //std::cout<<"number is"<<num<<std::endl; } else { if(checkMax(num, 10, c, INT_MAX)) num = num*10 +c; else { num = INT_MAX; break; } //std::cout<<"number is"<<num<<std::endl; } } return num; } }; 

Comentarios

• ¡Bienvenido a Code Review! Ha presentado una solución alternativa, pero no ‘ t revisó el código. Explique su razonamiento (cómo funciona su solución y por qué es mejor que la original) para que el autor y otros lectores puedan aprender de su proceso de pensamiento.
• el código utiliza un método, donde checkMin, donde no La multiplicación directa se realiza hasta que se valida el resultado. ser mayor que INT_MIN.