Ting du kan forbedre

Variabler / initialisering

• Hvor erklærer du multiplier? Jeg antar at siden det ikke er erklært innenfor metoden, blir det erklært som en global variabel. Prøv å unngå globale variabler.

  Problemet med globale variabler er at siden hver funksjon har tilgang til disse, blir det stadig vanskeligere å finne ut hvilke funksjoner som faktisk leser og skriver disse variablene.

  For å forstå hvordan applikasjonen fungerer, må du stort sett ta hensyn til hver funksjon som endrer den globale staten. Det kan gjøres, men etter hvert som applikasjonen vokser, vil det bli vanskeligere å være tilnærmet umulig (eller i det minste fullstendig bortkastet tid).

  Hvis du ikke stoler på globale variabler, vil du kan formidle tilstanden mellom forskjellige funksjoner etter behov. På den måten har du en mye bedre sjanse for å forstå hva hver funksjon gjør, da du ikke trenger å ta hensyn til den globale staten.

  Så i stedet for å bruke globale variabler, initialiser variablene i main(), og send dem som argumenter til funksjoner om nødvendig. I dette tilfellet ser jeg ikke behovet for at multiplier skal brukes utenfor funksjonen i det hele tatt, så hold det bare erklært innenfor funksjonen.

• sign skal være en int, og ikke en char .

Algoritme

• Akkurat nå implementerer du en komplisert og vanskelig å følge metoden for å konvertere et tegn til et tall. Den enkle måten er å få isdigit() til å gjøre det harde arbeidet for deg. Dette vil også hjelpe deg med å implementere TØRKE prinsipp .

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

  Se hvordan du har to sløyfer som gjør nesten identiske ting? Slik forenklet jeg alt dette ved å bruke isdigit().

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

  Du sløyfer gjennom tegnene i strengen så lenge de er sifre. For hver, legg til telleren du holder på – verdien du skal legge til er tegnetes heltall. Dette gjøres ved å trekke ASCII-verdien til "0" fra ascii-verdien til det aktuelle tallet.

• Merk at denne koden ikke «t håndtak overløp. Hvis du sender inn» 89384798719061231 «(som ikke får plass i en int), er resultatet udefinert. Løsningen er enkel nok, bruk bare en long long int for å redusere det. Vi har fortsatt problemer med ekstremt lange tall, men det er litt mer komplisert å fikse det slik at funksjonen fungerer som tiltenkt.

Dokumentasjon

• Hvor gikk alle kommentarene dine? En nyere utvikler ville rett og slett se på noe av koden din.

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

  Kommentarer kan virkelig hjelpe andre til å forstå koden din. Ikke gå overbord med dem, men du må balansere hvor mye av dem å legge inn i programmet.

Syntaks / styling

• Dette ser ut som en skrivefeil.

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

  Legg til et rom for klarhet.

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

• Du bør ikke endrer char* du godtar som parameter i funksjonen. Erklær derfor parameteren som konstant.

  int my_atoi(const char* pointer) 

• Bruk flere stenografiske operatorer.

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

Endelig kode

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

Kommentarer

• Du bør ikke ‘ t endre returtypene vilkårlig. atoi() returnerer tradisjonelt en int, så my_atoi() bør også. Hvis du vil analysere en long long, må du etterligne strtoll().
• isdigit(*c) er ikke definert for *c verdier mindre enn 0 (annet enn EOF). Bedre å while (isdigit((unsigned char) (*c) ))
• Misset hjørne: Når my_atoi() resultatet skal være LLONG_MIN, value += (int) (*c-'0'); er signert heltalloverløp (UB) når den prøver å danne LLONG_MAX + 1.
• Bruk isdigit er i det hele tatt feil, siden den ikke ‘ ikke har en relatert funksjon numeric_value. Derfor, hvis tegnsettet ditt har to sifreområder (0 til 9 og ٠ til ٩), blir indikasjonstallene feil analysert. Bare hold deg til '0' <= c && c <= '9' for å være trygg. Dette unngår også den udefinerte oppførselen fra å bruke ctype-funksjonen feil.
• Du savnet et viktig poeng da du skrev » ASCII-verdi på ‘ 0 ‘ » : der ‘ div ingenting som sier at vertskarakteren trenger å være ASCII (bare at 0..9 er sammenhengende). Det er ‘ hvorfor du skriver '0' i stedet for et kodingsspesifikt kodepunktnummer.

[Rediger]

Med unntak av oppførselen ved feil, er atoi() ekvivalent til (int)strtol(nptr, (char **)NULL, 10). strtol() godtar ledende hvite rom. OP «s my_atoi(char* pointer) gjør ikke det. For å avhjelpe:

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

Nedenfor beskrives en god måte å håndtere INT_MIN.

OTOH, å levere verdier utenfor [INT_MIN...INT_MAX] er ikke definert av C-spesifikasjonen, så noen forenklinger kan være hadde. Se langt nedenfor.

Når en streng representerer INT_MIN, (la oss anta 32-bit int) som "-2147483648", koden løper inn i int overløp og prøver å beregne 2147483648. En enkel måte å løse dette på er i stedet for å finne den positive verdien og deretter negere den, omfavne den negative siden av ting. Ved å gjøre løveandelen av matematikken i INT_MIN til 0 -området, unngår vi UB. Ned-siden: noen synes denne tilnærmingen er mer utfordrende å følge.

Å gå til et bredere heltall eller unsigned det er ikke alltid mulig da heltallstørrelsen på «tekst- -> heltall «rutine kan være maksimal størrelse. Strengt tatt har unsigned ikke alltid et bredere positivt område enn int. I alle fall kan all matematikk håndteres med ønsket signert heltallstørrelse uten å ty til andre typer.

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

Merknader:

pointer%48 er forvirrende. Hva er spesielt med 48? Hvis du mener "0", så bruk pointer % "0".

«streng:» 232-19 «. Hva skal jeg gjøre da? » Anbefaler å stoppe konvertering ved «232» og returnere verdien 232. Kunne sette errno, men den typiske atoi() funksjonen gjør ikke for mye feilhåndtering.

Ved overløp kan innstilling av errno skje, men igjen, typisk atoi() -funksjonen gjør ikke for mye feilhåndtering. Foreslå enkel retur INT_MAX eller INT_MIN.

Hvis du vil ha bedre feilhåndtering, kan du endre til noe som følgende og angi en feilstatus.

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

eller plassering der ting endte. Hvis dette er bra, endte de på "\0".

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

[Rediger] Forenklet: Fjernet deteksjon utenfor rekkevidde som C-spesifikasjon tillater det. «Hvis verdien av resultatet ikke kan vises, er oppførselen udefinert.

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

Kommentarer

• INT_MIN/10 og INT_MIN%10 krever C99-oppførsel.

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

Hvorfor henvise til «peker» tre ganger? Én gang er nok:

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

Kommentarer

• Velkommen til Code Review, ditt første svar ser bra ut , kos deg! Selv om jeg lurer på om det gjør en forskjell i den genererte koden.

hvis du er ok med rekursjon da kunne koden forkortes til en under

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

Stakkutmattelse kunne reduseres ved -foptimize-sibling-calls kompilatorflagg, det å være støttes av både GCC- og Clang-kompilatorer.

Oppdatering:

Som nevnt av Roland Illig implementering håndterer ikke misdannede innspill. Hvis det ønskes følg atoi semantikk , bør neste kode være fine _{ikke glem sett Compile Options til en i kommentarer} .

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

Dette er fortsatt chux «s kode der sløyfene er erstattet med rekursjon

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

Kommentarer

• Test case: buf = malloc(65536); buf[0] = '\0'; my_atoi(buf) vil trolig krasje.
• Test case: bufsize = 1 << 20; buf = malloc(bufsize); memset(buf, '0', bufsize); buf[bufsize - 1] = '\0'; my_atoi(buf) vil ta veldig lang tid.

For en øvelse i leetcode , skrev følgende impl: 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; } }; 

Kommentarer

• Velkommen til Code Review! Du har presentert en alternativ løsning, men har ikke ‘ t gjennomgått koden. Vennligst forklar resonnementet ditt (hvordan løsningen din fungerer og hvorfor den er bedre enn originalen), slik at forfatteren og andre lesere kan lære av tankeprosessen din.
• koden bruker en metode, der checkMin, der ingen direkte multiplikasjon utføres til resultatet er validert. å være større enn INT_MIN.