Lucruri pe care le-ați putea îmbunătăți

Variabile / Inițializare

• Unde declarați multiplier? Presupun că, deoarece nu este declarat în cadrul metodei, este declarat ca o variabilă globală. Încercați să evitați variabilele globale.

  Problema cu variabilele globale este că, din moment ce fiecare funcție are acces la acestea, devine din ce în ce mai greu să vă dați seama ce funcții citesc și scriu aceste variabile.

  Pentru a înțelege cum funcționează aplicația, trebuie să țineți cont de fiecare funcție care modifică starea globală. Acest lucru se poate face, dar pe măsură ce aplicația crește, va deveni mai dificilă până la a fi practic imposibilă (sau cel puțin o pierdere completă de timp).

  Dacă nu vă bazați pe variabile globale, poate trece starea între diferite funcții, după cum este necesar. În acest fel, aveți șanse mult mai mari de a înțelege ce face fiecare funcție, deoarece nu trebuie să țineți cont de starea globală.

  Deci, în loc să utilizați variabile globale, inițializați variabilele din main() și transmiteți-le ca argumente funcțiilor, dacă este necesar. În acest caz, nu văd necesitatea ca multiplier să fie utilizat în afara funcției, așa că pur și simplu păstrați-l declarat în cadrul funcției.

• sign ar trebui să fie un int și nu un char .

Algoritm

• Chiar acum implementați o metodă complicată și greu de urmat de convertire a unui caracter într-un număr. Modul ușor este ca isdigit() să facă munca grea pentru dvs. Acest lucru vă va ajuta, de asemenea, să implementați Principiul 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; } 

  Vedeți cum aveți două bucle care fac lucruri aproape identice? Iată cum am simplificat toate acestea folosind isdigit().

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

  Parcurgeți caracterele din șir, atâta timp cât acestea sunt cifre. Pentru fiecare, adăugați la contor vă păstrați – valoarea de adăugat este valoarea întreagă a caracterului. Acest lucru se realizează scăzând valoarea ASCII a "0" din valoarea ascii a cifrei în cauză.

• Rețineți că acest cod nu „T handle overflow. Dacă treceți în” 89384798719061231 „(care a câștigat” t fit într-un int), rezultatul este nedefinit. Remedierea este suficient de simplă, trebuie doar să utilizați un long long int pentru a atenua acest lucru. Vom avea în continuare probleme pentru numerele extrem de lungi, dar remedierea acesteia astfel încât funcția să funcționeze conform intenției este puțin mai complicată.

Documentare

• Unde s-au îndreptat toate comentariile dvs.? Un dezvoltator mai nou ar uita pur și simplu o parte din codul dvs.

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

  Comentariile pot contribui cu adevărat la a-i ajuta pe ceilalți să înțeleagă codul dvs. Totuși, nu treceți peste bord cu ei, va trebui să echilibrați cât să le introduceți în programul dvs.

Sintaxă / Styling

• Aceasta pare a fi o greșeală de scriere.

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

  Adăugați un spațiu pentru claritate.

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

• Ar trebui să nu modificați char* pe care îl acceptați ca parametru în funcție. Prin urmare, declarați parametrul ca fiind constant.

  int my_atoi(const char* pointer) 

• Utilizați mai mulți operatori de stenogramă.

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

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

Comentarii

• Nu ar trebui să ‘ să schimbați în mod arbitrar tipurile de returnare. atoi() în mod tradițional returnează un int, așa că și my_atoi() ar trebui. Dacă doriți să analizați un long long, atunci emulați strtoll().
• isdigit(*c) nu este definit pentru *c valori mai mici de 0 (altele decât EOF). Mai bine să while (isdigit((unsigned char) (*c) ))
• Colț ratat: când my_atoi() rezultatul ar trebui să fie LLONG_MIN, value += (int) (*c-'0'); este semn de depășire a numărului întreg (UB) în timp ce încearcă să formeze LLONG_MAX + 1.
• Folosind isdigit este deloc greșit, deoarece nu are ‘ o funcție înrudită numeric_value. Prin urmare, dacă setul dvs. de caractere are două game de cifre (de la 0 la 9 și de la ٠ la ٩), numerele Indic vor fi analizate greșit. Pur și simplu rămâneți la '0' <= c && c <= '9' pentru a fi în siguranță. Acest lucru evită, de asemenea, comportamentul nedefinit de la utilizarea incorectă a funcției ctype.
• Ați ratat un punct important când ați scris ” valoarea ASCII a ‘ 0 ‘ ” : acolo ‘ nimic nu spune că setul de caractere gazdă nu trebuie să fie ASCII (numai că 0..9 sunt contigue). Acesta este ‘ motivul pentru care scrieți '0' mai degrabă decât un număr de punct de cod specific codării.

[Edit]

Cu excepția comportamentului la eroare, atoi() este echivalent la (int)strtol(nptr, (char **)NULL, 10). strtol() acceptă spațiul alb principal. OP „s my_atoi(char* pointer) nu. Pentru a remedia:

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

Mai jos descrie un mod bun de a gestiona INT_MIN.

OTOH, predarea valorilor în afara [INT_MIN...INT_MAX] nu este definită de specificația C, deci unele simplificări pot fi had. Vedeți mult mai jos.

Când un șir reprezintă INT_MIN, (să presupunem că pe 32 de biți int) cum ar fi "-2147483648", codul rulează în int overflow încercând să ajungă la calcularea 2147483648. O modalitate simplă de a rezolva acest lucru este mai degrabă decât să găsiți valoarea pozitivă și apoi să o negați, să îmbrățișați partea negativă a lucrurilor. Făcând cea mai mare parte a matematicii din INT_MIN până la 0, evităm UB. Dezavantaj: unii consideră că această abordare este mai dificilă de urmat.

Dacă mergeți la un număr întreg mai larg sau unsigned nu este întotdeauna posibil ca dimensiunea întregului textului -> rutina „întreg” poate avea dimensiunea maximă. Strict vorbind, unsigned nu are întotdeauna un interval pozitiv mai larg decât int. În orice caz, toate matematica poate fi tratată la dimensiunea întregului semnat dorit fără a recurge la alte tipuri.

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

Note:

pointer%48 este confuz. Ce este special la 48? Dacă vrei să spui "0", atunci folosește pointer % "0".

„șir:” 232-19 „. Ce ar trebui să faci atunci? ” Recomandați oprirea conversiei la „232” și returnarea valorii 232. Putea seta errno, dar atoi() funcția nu gestionează prea mult erorile.

La depășire, setarea errno, s-ar putea întâmpla, dar din nou, tipic atoi() nu face prea multe gestionări de erori. Sugerați o revenire simplă INT_MAX sau INT_MIN.

Dacă doriți o mai bună gestionare a erorilor, treceți la ceva precum următoarele și setați o stare de eroare.

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

sau locație unde s-au terminat lucrurile. Dacă acestea sunt bune, acestea s-au încheiat la "\0".

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

[Edit] Simplificat: Eliminat detectarea în afara intervalului, deoarece specificațiile C permit acest lucru. „Dacă valoarea rezultatului nu poate fi reprezentată, comportamentul este nedefinit.

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

Comentarii

• INT_MIN/10 și INT_MIN%10 necesită un comportament C99.

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

De ce să ne referim la „pointer” de trei ori? O singură dată este suficient:

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

Comentarii

• Bine ați venit la Code Review, primul dvs. răspuns arată bine , sedere placuta! Deși mă întreb dacă aceasta face diferența în codul generat.

dacă ești bine cu recursivitatea atunci codul ar putea fi scurtat la unul mai jos

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

Epuizarea stivei ar putea fi atenuată de -foptimize-sibling-calls steagul compilatorului, acesta fiind acceptat atât de compilatoarele GCC, cât și de cele Clang.

Actualizare:

După cum sa menționat prin Roland Illig implementarea nu gestionează intrările malformate. Dacă se dorește în continuare urmăriți atoi semantică atunci următorul cod ar trebui să fie bine _{nu „uitați setați Compile Options la unul în comentarii} .

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

Acesta este încă codul chux „în care buclele sunt înlocuite cu recursivitate

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

Comentarii

• Test case: buf = malloc(65536); buf[0] = '\0'; my_atoi(buf) probabil se va bloca.
• Test test: bufsize = 1 << 20; buf = malloc(bufsize); memset(buf, '0', bufsize); buf[bufsize - 1] = '\0'; my_atoi(buf) va dura foarte mult timp.

Pentru un exercițiu în leetcode , a scris următorul impl: cod CPI 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; } }; 

Comentarii

• Bun venit la Code Review! Ați prezentat o soluție alternativă, dar ‘ nu ați examinat codul. Vă rugăm să explicați raționamentul dvs. (cum funcționează soluția dvs. și de ce este mai bună decât originalul), astfel încât autorul și ceilalți cititori să poată învăța din procesul dvs. de gândire.
• codul folosește o metodă, unde checkMin, unde nu multiplicarea directă se realizează până la validarea rezultatului. să fie mai mare decât INT_MIN.