Ting, du kan forbedre

Variabler / initialisering

• Hvor erklærer du multiplier? Jeg antager, at da det ikke er deklareret inden for metoden, erklæres det som en global variabel. Prøv at undgå globale variabler.

  Problemet med globale variabler er, at da hver funktion har adgang til disse, bliver det stadig sværere at finde ud af, hvilke funktioner der faktisk læser og skriver disse variabler.

  For at forstå, hvordan applikationen fungerer, skal du stort set tage højde for enhver funktion, der ændrer den globale tilstand. Det kan gøres, men efterhånden som applikationen vokser, bliver det sværere til næsten at være umuligt (eller i det mindste et fuldstændigt spild af tid).

  Hvis du ikke stoler på globale variabler, kan videregive tilstand mellem forskellige funktioner efter behov. På den måde har du en meget bedre chance for at forstå, hvad hver funktion gør, da du ikke behøver at tage den globale stat med i betragtning.

  Så i stedet for at bruge globale variabler, initialiser variablerne i main(), og send dem som argumenter til funktioner, hvis det er nødvendigt. I dette tilfælde ser jeg ikke behovet for, at multiplier overhovedet skal bruges uden for funktionen, så hold det kun erklæret inden for funktionen.

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

Algoritme

• Lige nu implementerer du en kompliceret og svær at følge metoden til at konvertere et tegn til et tal. Den nemme måde er at have isdigit() til at gøre det hårde arbejde for dig. Dette vil også hjælpe dig med at implementere TØRRE princip .

  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øjfer, der laver næsten identiske ting? Her er hvordan jeg forenklet alt dette ved at bruge isdigit().

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

  Du løber gennem tegnene i strengen, så længe de er cifre. For hver tilføj til tælleren du holder på – værdien, der skal tilføjes, er karakterens heltal. Dette gøres ved at trække ASCII-værdien af "0" fra ascii-værdien for det pågældende ciffer.

• Bemærk, at denne kode ikke “t håndterer overløb. Hvis du sender” 89384798719061231 “(som ikke passer i en int), er resultatet udefineret. Løsningen er enkel nok, brug bare en long long int til at afbøde mod det. Vi har stadig problemer med ekstremt lange numre, men det er lidt mere kompliceret at rette det, så funktionen fungerer efter hensigten.

Dokumentation

• Hvor gik alle dine kommentarer hen? En nyere udvikler ville simpelthen se på noget af din kode.

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

  Kommentarer kan virkelig gå langt i at hjælpe andre med at forstå din kode. Gå dog ikke overbord med dem, du bliver nødt til at afbalancere hvor meget af dem til at sætte i dit program.

Syntaks / styling

• Dette ligner en tastefejl.

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

  Tilføj et mellemrum for klarhed.

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

• Du skal ikke skal du ændre din char* du accepterer som parameter i funktionen. Erklær derfor parameteren som konstant.

  int my_atoi(const char* pointer) 

• Brug 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 ændre returtyper vilkårligt. atoi() returnerer traditionelt en int, så my_atoi() skal også. Hvis du vil analysere en long long, skal du efterligne strtoll().
• isdigit(*c) er ikke defineret for *c værdier mindre end 0 (bortset fra EOF). Bedre at while (isdigit((unsigned char) (*c) ))
• Ubesvaret hjørne: Når my_atoi() resultatet skal være LLONG_MIN, value += (int) (*c-'0'); er underskrevet heltalsoverløb (UB), da det forsøger at danne LLONG_MAX + 1.
• Brug af isdigit er overhovedet forkert, da den ikke ‘ ikke har en relateret funktion numeric_value. Derfor, hvis dit tegnsæt har to cifreområder (0 til 9 og ٠ til ٩), bliver indikatornumrene forkert forkert. Bare hold dig til '0' <= c && c <= '9' for at være sikker. Dette undgår også den udefinerede adfærd ved at bruge ctype-funktionen forkert.
• Du gik glip af et vigtigt punkt, da du skrev ” ASCII-værdi på ‘ 0 ‘ ” : der ‘ s intet, der siger værts tegnsæt skal være ASCII (kun at 0..9 er sammenhængende). Derfor ‘ hvorfor du skriver '0' snarere end et kodningsspecifikt kodepunktnummer.

[Rediger]

Bortset fra adfærden ved fejl er atoi() ækvivalent til (int)strtol(nptr, (char **)NULL, 10). strtol() accepterer førende hvidt rum. OP “s my_atoi(char* pointer) gør ikke. For at afhjælpe:

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

Nedenstående beskriver en god måde at håndtere INT_MIN.

OTOH, aflevering af værdier uden for [INT_MIN...INT_MAX] er ikke defineret af C-specifikationen, så nogle forenklinger kan være Se langt nedenfor.

Når en streng repræsenterer INT_MIN, (lad os antage 32-bit int) såsom "-2147483648", koden løber ind i int overløb forsøger at komme til at beregne 2147483648. En enkel måde at løse dette på er snarere end at finde den positive værdi og derefter negere den, omfavn negativ side af tingene. Ved at udføre den største del af matematikken i INT_MIN til 0 -området undgår vi UB. Nedad: nogle finder denne fremgangsmåde mere udfordrende at følge.

At gå til et bredere heltal eller unsigned det er ikke altid muligt, da heltalstørrelsen på “tekst- -> heltal “rutine kan være den maksimale størrelse. Strengt taget har unsigned ikke altid et bredere positivt interval end int. Under alle omstændigheder kan al matematik håndteres i den ønskede underskrevne heltalstørrelse uden at 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; } 

Bemærkninger:

pointer%48 er forvirrende. Hvad er specielt ved 48? Hvis du mener "0", så brug pointer % "0".

“streng:” 232-19 “. Hvad skal jeg gøre så? ” Anbefaler at stoppe konvertering ved “232” og returnere værdien 232. Kunne indstille errno, men den typiske atoi() funktion gør ikke for meget fejlhåndtering.

Ved overløb kan indstilling af errno ske, men igen, typisk atoi() -funktionen gør ikke for meget fejlhåndtering. Foreslå enkel returnering INT_MAX eller INT_MIN.

Hvis du vil have bedre fejlhåndtering, skal du ændre til noget som det følgende og indstil en fejlstatus.

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

eller placering hvor tingene sluttede. Hvis dette er godt, sluttede de ved "\0".

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

[Rediger] Forenklet: Fjernet uden for rækkevidde som C-spec tillader det. “Hvis værdien af resultatet ikke kan repræsenteres, er adfærden udefineret.

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 kræver C99-opførsel.

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

Hvorfor henvise til “pointer” tre gange? Én gang er nok:

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

Kommentarer

• Velkommen til Code Review, dit første svar ser godt ud , Nyd dit ophold! Selvom jeg spekulerer på, om det gør en forskel i den genererede kode.

hvis du er ok med rekursion så 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; 

Stakudmattelse kunne afhjælpes ved -foptimize-sibling-calls compiler-flag, da understøttet af både GCC og Clang compilers.

Opdatering:

Som bemærket af Roland Illig implementering håndterer ikke misdannet input. Hvis det ønskes nøje, skal du følge atoi semantik , så skal næste kode være fint _{glem ikke sæt 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 stadig chux “s kode, hvor sløjfer erstattes med rekursion

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 sandsynligvis gå ned.
• Test case: bufsize = 1 << 20; buf = malloc(bufsize); memset(buf, '0', bufsize); buf[bufsize - 1] = '\0'; my_atoi(buf) vil tage meget lang tid.

Til 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 præsenteret en alternativ løsning, men har ikke ‘ t gennemgået koden. Forklar din argumentation (hvordan din løsning fungerer, og hvorfor den er bedre end originalen), så forfatteren og andre læsere kan lære af din tankeproces.
• koden bruger en metode, hvor checkMin, hvor ingen direkte multiplikation udføres, indtil resultatet er valideret. at være større end INT_MIN.