Ce que vous pourriez améliorer

Variables / Initialisation

• Où déclarez-vous multiplier? Je suppose que puisquil nest pas déclaré dans la méthode, il est déclaré comme une variable globale. Essayez déviter les variables globales.

  Le problème avec les variables globales est que puisque chaque fonction y a accès, il devient de plus en plus difficile de déterminer quelles fonctions lisent et écrivent réellement ces variables.

  Pour comprendre le fonctionnement de lapplication, il faut à peu près prendre en compte chaque fonction qui modifie létat global. Cela peut être fait, mais au fur et à mesure que l’application se développe, cela deviendra plus difficile au point d’être pratiquement impossible (ou du moins une perte de temps totale).

  Si vous ne vous fiez pas aux variables globales, vous peut transmettre un état entre différentes fonctions si nécessaire. De cette façon, vous avez une bien meilleure chance de comprendre ce que fait chaque fonction, car vous navez pas besoin de prendre en compte létat global.

  Donc, au lieu dutiliser variables globales, initialisez les variables dans main(), et passez-les comme arguments aux fonctions si nécessaire. Dans ce cas, je ne vois pas du tout la nécessité dutiliser multiplier en dehors de la fonction, alors gardez-le simplement déclaré dans la fonction.

• sign doit être un int et non un char .

Algorithme

• En ce moment, vous implémentez une méthode compliquée et difficile à suivre pour convertir un caractère en nombre. Le moyen le plus simple est de demander à isdigit() de faire le travail à votre place. Cela vous aidera également à mettre en œuvre le Principe DRY .

  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; } 

  Voyez comment deux boucles font des choses presque identiques? Voici comment jai simplifié tout cela en utilisant isdigit().

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

  Vous parcourez les caractères de la chaîne tant quil sagit de chiffres. Pour chacun deux, ajoutez au compteur que vous gardez – la valeur à ajouter est la valeur entière du caractère. Ceci est fait en soustrayant la valeur ASCII de "0" de la valeur ascii du chiffre en question.

• Notez que ce code ne « t handle overflow. Si vous passez » 89384798719061231 « (qui ne » t rentrera pas dans un int), le résultat est indéfini. Le correctif est assez simple, utilisez simplement un long long int pour atténuer cela. Nous aurons encore des problèmes pour les nombres extrêmement longs, mais résoudre ce problème pour que la fonction fonctionne comme prévu est un peu plus compliqué.

Documentation

• Où sont passés tous vos commentaires? Un développeur plus récent ne ferait que regarder une partie de votre code.

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

  Les commentaires peuvent vraiment aider les autres à comprendre votre code. Nallez pas trop loin avec eux, vous devrez trouver un équilibre les mettre dans votre programme.

Syntaxe / Style

• Cela ressemble à une faute de frappe.

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

  Ajoutez un espace pour plus de clarté.

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

• Vous ne devez pas modifie votre char* que vous acceptez comme paramètre dans la fonction. Par conséquent, déclarez le paramètre comme constant.

  int my_atoi(const char* pointer) 

• Utilisez plus dopérateurs de raccourcis.

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

Code 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 } 

Commentaires

• Vous ne devriez pas ‘ changer les types de retour de manière arbitraire. atoi() renvoie traditionnellement un int, donc my_atoi() devrait également le faire. Si vous souhaitez analyser un long long, émulez strtoll().
• isdigit(*c) nest pas défini pour les valeurs *c inférieures à 0 (autres que EOF). Mieux vaut while (isdigit((unsigned char) (*c) ))
• Coin manqué: lorsque my_atoi() le résultat doit être LLONG_MIN, value += (int) (*c-'0'); est un dépassement dentier signé (UB) alors quil tente de former LLONG_MAX + 1.
• Utilisation de isdigit est faux du tout, car il na ‘ pas de fonction associée numeric_value. Par conséquent, si votre jeu de caractères comporte deux plages de chiffres (0 à 9 et ٠ à ٩), les nombres indiens seront mal analysés. Restez fidèle à '0' <= c && c <= '9' pour être sûr. Cela évite également que le comportement non défini nutilise la fonction ctype de manière incorrecte.
• Vous avez manqué un point important lorsque vous avez écrit  » la valeur ASCII de ‘ 0 ‘  » : il y a ‘ Il ny a rien qui indique que le jeu de caractères de lhôte doit être ASCII (seuls les 0..9 sont contigus). Cest ‘ pourquoi vous écrivez '0' plutôt quun numéro de point de code spécifique à lencodage.

[Modifier]

À lexception du comportement en cas derreur, atoi() est équivalent à (int)strtol(nptr, (char **)NULL, 10). strtol() accepte les espaces de début. OP « s my_atoi(char* pointer) ne le fait pas. Pour y remédier:

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

Ce qui suit décrit un bon moyen de gérer INT_MIN.

OTOH, la remise de valeurs en dehors de [INT_MIN...INT_MAX] nest pas définie par la spécification C, donc certaines simplifications peuvent être had. Voir loin ci-dessous.

Quand une chaîne représente INT_MIN, (supposons que 32 bits int) tel que "-2147483648", le code se heurte à un int débordement en essayant de calculer 2147483648. Un moyen simple de résoudre ce problème est plutôt que de trouver la valeur positive puis de la nier, adoptez le côté négatif des choses. En faisant la part du lion des calculs dans la plage INT_MIN à 0, nous évitons UB. Inconvénient: certains trouvent cette approche plus difficile à suivre.

Passer à un entier plus large ou unsigned nest pas toujours possible car la taille entière de « text- -> integer « routine peut être la taille maximale. À proprement parler, unsigned na pas toujours une plage positive plus large que int. Dans tous les cas, tous les calculs peuvent être traités à la taille entière signée souhaitée sans recourir à dautres types.

#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; } 

Remarques:

pointer%48 est déroutant. Quelle est la particularité du 48? Si vous voulez dire "0", utilisez pointer % "0".

« string: » 232-19 « . Que dois-je faire alors?  » Il est recommandé darrêter la conversion à « 232 » et de renvoyer la valeur 232. Pourrait définir errno, mais le typique atoi() ne gère pas trop les erreurs.

En cas de dépassement de capacité, la configuration de errno peut se produire, mais encore une fois, atoi() ne gère pas trop les erreurs. Suggérer un retour simple INT_MAX ou INT_MIN.

Si vous voulez une meilleure gestion des erreurs, changez pour quelque chose comme ce qui suit et définir un état derreur.

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

ou emplacement là où les choses se sont terminées. Si cela est bon, ils se sont terminés par "\0".

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

[Edit] Simplified: Removed détection hors de portée, comme le permet la spécification C. « Si la valeur du résultat ne peut pas être représentée, le comportement nest pas défini.

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; } 

Commentaires

• INT_MIN/10 et INT_MIN%10 nécessitent un comportement C99.

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

Pourquoi déréférencer « pointer » trois fois? Une seule fois suffit:

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

Commentaires

• Bienvenue dans Code Review, votre première réponse semble bonne , Profitez de votre séjour! Bien que je me demande si cela fait une différence dans le code généré.

si vous êtes daccord avec la récursivité alors le code pourrait être raccourci à celui ci-dessous

#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; 

Lépuisement de la pile pourrait être atténué par lindicateur du compilateur -foptimize-sibling-calls, cela étant pris en charge par les compilateurs GCC et Clang.

Mise à jour:

Comme indiqué par la mise en œuvre Roland Illig ne gère pas les entrées malformées. Si vous le souhaitez, suivez de près la atoi sémantique , alors le code suivant doit être bien _{noubliez pas de définir Compile Options sur un dans les commentaires} .

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); } 

Il sagit toujours du code de chux « où les boucles sont remplacées par la récursivité

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")) } 

Commentaires

• Cas de test: buf = malloc(65536); buf[0] = '\0'; my_atoi(buf) plantera probablement.
• Cas de test: bufsize = 1 << 20; buf = malloc(bufsize); memset(buf, '0', bufsize); buf[bufsize - 1] = '\0'; my_atoi(buf) prendra très temps.

Pour un exercice de leetcode , écrit impl suivant: atoi cpp code

 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; } }; 

Commentaires

• Bienvenue dans Code Review! Vous avez présenté une solution alternative, mais n’a ‘ pas examiné le code. Veuillez expliquer votre raisonnement (comment votre solution fonctionne et pourquoi elle est meilleure que loriginal) afin que lauteur et les autres lecteurs puissent apprendre de votre processus de réflexion.
• le code utilise une méthode, où checkMin, où non la multiplication directe est effectuée jusquà ce que le résultat soit validé. être supérieur à INT_MIN.