Cose che potresti migliorare

Variabili / Inizializzazione

• Dove dichiari multiplier? Presumo che, poiché non è dichiarato allinterno del metodo, viene dichiarato come una variabile globale. Cerca di evitare le variabili globali.

  Il problema con le variabili globali è che poiché ogni funzione ha accesso a queste, diventa sempre più difficile capire quali funzioni effettivamente leggono e scrivono queste variabili.

  Per capire come funziona lapplicazione, devi praticamente prendere in considerazione ogni funzione che modifica lo stato globale. Questo può essere fatto, ma man mano che lapplicazione cresce diventerà più difficile al punto da essere virtualmente impossibile (o almeno una completa perdita di tempo).

  Se non fai affidamento sulle variabili globali, può trasferire lo stato tra le diverse funzioni secondo necessità. In questo modo hai molte più possibilità di capire cosa fa ciascuna funzione, poiché non è necessario tenere conto dello stato globale.

  Quindi invece di usare variabili globali, inizializza le variabili in main() e, se necessario, passale come argomenti alle funzioni. In questo caso, non vedo la necessità di multiplier da utilizzare al di fuori della funzione, quindi tienilo semplicemente dichiarato allinterno della funzione.

• sign dovrebbe essere un int e non un char .

Algoritmo

• In questo momento stai implementando un metodo complicato e difficile da seguire per convertire un carattere in un numero. Il modo più semplice è lasciare che isdigit() faccia il duro lavoro per te. Questo ti aiuterà anche a implementare il principio 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; } 

  Vedi come hai due cicli che fanno cose quasi identiche? Ecco come ho semplificato tutto usando isdigit().

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

  Ripeti i caratteri nella stringa purché siano cifre. Per ognuno, aggiungi al contatore stai mantenendo – il valore da aggiungere è il valore intero del carattere. Questo viene fatto sottraendo il valore ASCII di "0" dal valore ascii della cifra in questione.

• Nota che questo codice non “t handle overflow. Se passi” 89384798719061231 “(che non” t si adatta a un int), il risultato non è definito. La correzione è abbastanza semplice, basta usare un long long int per mitigare il problema. Avremo ancora problemi per numeri estremamente lunghi, ma risolverli in modo che la funzione funzioni come previsto è un po più complicato.

Documentazione

• Dove sono finiti tutti i tuoi commenti? Uno sviluppatore più recente si limiterebbe a guardare un po del tuo codice.

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

  I commenti possono davvero fare molto per aiutare gli altri a capire il tuo codice. Non esagerare con loro, però, dovrai bilanciare quanto di da inserire nel tuo programma.

Sintassi / Stile

• Sembra un errore di battitura.

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

  Aggiungi uno spazio per maggiore chiarezza.

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

• Non dovresti sta modificando il tuo char* che accetti come parametro nella funzione. Pertanto, dichiara il parametro come costante.

  int my_atoi(const char* pointer) 

• Utilizza più operatori abbreviati.

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

Codice finale

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

Commenti

• Non dovresti ‘ cambiare i tipi di ritorno arbitrariamente. atoi() tradizionalmente restituisce un int, quindi anche my_atoi() dovrebbe. Se desideri analizzare un long long, emula strtoll().
• isdigit(*c) non è definito per *c valori inferiori a 0 (diverso da EOF). Meglio while (isdigit((unsigned char) (*c) ))
• Angolo mancato: quando my_atoi() il risultato dovrebbe essere LLONG_MIN, value += (int) (*c-'0'); è un intero overflow con segno (UB) mentre cerca di formare LLONG_MAX + 1.
• Utilizzo di isdigit è completamente sbagliato, poiché ‘ non ha una funzione correlata numeric_value. Pertanto, se il set di caratteri ha due intervalli di cifre (da 0 a 9 e da ٠ a ٩), i numeri indiani verranno interpretati in modo errato. Attenersi a '0' <= c && c <= '9' per sicurezza. Questo evita anche che il comportamento indefinito utilizzi la funzione ctype in modo errato.
• Hai perso un punto importante quando hai scritto ” valore ASCII di ‘ 0 ‘ ” : lì ‘ Non cè niente che dica che il set di caratteri host deve essere ASCII (solo che 0..9 sono contigui). Questo è ‘ perché scrivi '0' invece di un numero di codepoint specifico della codifica.

[Modifica]

Fatta eccezione per il comportamento in caso di errore, atoi() è equivalente a (int)strtol(nptr, (char **)NULL, 10). strtol() accetta spazi vuoti iniziali. OP “s my_atoi(char* pointer) non funziona. Per rimediare:

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

Quanto segue descrive un buon modo per gestire INT_MIN.

OTOH, la consegna di valori al di fuori di [INT_MIN...INT_MAX] non è definita dalla specifica C, quindi alcune semplificazioni possono essere aveva. Vedi molto di seguito.

Quando una stringa rappresenta INT_MIN, (supponiamo che 32 bit int) come "-2147483648", il codice viene eseguito in int overflow nel tentativo di ottenere il calcolo 2147483648. Un modo semplice per risolverlo è piuttosto che trovare il valore positivo e poi negarlo, abbracciare il lato negativo delle cose. Facendo la parte del leone nella matematica nellintervallo da INT_MIN a 0, evitiamo UB. Lato negativo: alcuni trovano questo approccio più difficile da seguire.

Passare a un numero intero più ampio o unsigned non è sempre possibile come dimensione intera di “text- -> intero “routine può essere la dimensione massima. In senso stretto, unsigned non ha sempre un intervallo positivo più ampio di int. In ogni caso, tutta la matematica può essere gestita alla dimensione di interi con segno desiderata senza ricorrere ad altri tipi.

#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 crea confusione. Cosha di speciale 48? Se intendi "0", utilizza pointer % "0".

“stringa:” 232-19 “. Cosa devo fare allora? ” Consiglia di interrompere la conversione a “232” e restituire il valore 232. Potrebbe impostare errno, ma il tipico atoi() la funzione non gestisce troppi errori.

In caso di overflow, potrebbe verificarsi limpostazione di errno, ma ancora una volta, atoi() non gestisce troppi errori. Suggerisci una semplice restituzione di INT_MAX o INT_MIN.

Se desideri una migliore gestione degli errori, cambia in qualcosa di simile al seguente e impostare uno stato di errore.

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

o posizione dove sono finite le cose. Se questo è corretto, è terminato con "\0".

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

[Modifica] Semplificato: rimosso rilevamento fuori intervallo come consentito dalle specifiche C. “Se il valore del risultato non può essere rappresentato, il comportamento è indefinito.

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

Commenti

• INT_MIN/10 e INT_MIN%10 richiedono il comportamento C99.

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

Perché de-referenziare “pointer” tre volte? Una volta è sufficiente:

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

Commenti

• Benvenuto in Code Review, la tua prima risposta sembra buona , goditi la permanenza! Anche se mi chiedo se fa la differenza nel codice generato.

se sei daccordo con la ricorsione quindi il codice potrebbe essere abbreviato in uno di seguito

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

Lesaurimento dello stack potrebbe essere mitigato dal -foptimize-sibling-calls flag del compilatore, ovvero supportato da entrambi i compilatori GCC e Clang.

Aggiornamento:

Come indicato Limplementazione di Roland Illig non gestisce input errati. Se si desidera seguire strettamente la atoi semantica , il codice successivo dovrebbe essere fine _{non dimenticare di impostare Compile Options su uno nei commenti} .

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

Questo è ancora il codice chux “in cui i cicli vengono sostituiti con la ricorsione

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

Commenti

• Scenario di test: buf = malloc(65536); buf[0] = '\0'; my_atoi(buf) probabilmente andrà in crash.
• Scenario di test: bufsize = 1 << 20; buf = malloc(bufsize); memset(buf, '0', bufsize); buf[bufsize - 1] = '\0'; my_atoi(buf) richiederà molto tempo.

Per un esercizio in leetcode , ha scritto quanto segue impl: codice 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; } }; 

Commenti

• Benvenuto in Code Review! Hai presentato una soluzione alternativa, ma non hai ‘ rivisto il codice. Spiega il tuo ragionamento (come funziona la tua soluzione e perché è migliore delloriginale) in modo che lautore e gli altri lettori possano imparare dal tuo processo di pensiero.
• il codice utilizza un metodo, dove checkMin, dove no la moltiplicazione diretta viene eseguita finché il risultato non viene convalidato. essere maggiore di INT_MIN.