Dinge, die Sie verbessern könnten

Variablen / Initialisierung

• Wo deklarieren Sie multiplier? Ich gehe davon aus, dass es als globale Variable deklariert wird, da es nicht innerhalb der Methode deklariert ist. Versuchen Sie, globale Variablen zu vermeiden.

  Das Problem mit globalen Variablen besteht darin, dass es zunehmend schwieriger wird, herauszufinden, welche Funktionen diese Variablen tatsächlich lesen und schreiben, da jede Funktion Zugriff auf diese hat.

  Um zu verstehen, wie die Anwendung funktioniert, müssen Sie so ziemlich jede Funktion berücksichtigen, die den globalen Status ändert. Dies ist möglich, aber wenn die Anwendung wächst, wird es schwieriger, praktisch unmöglich (oder zumindest eine völlige Zeitverschwendung) zu werden.

  Wenn Sie sich nicht auf globale Variablen verlassen, werden Sie Auf diese Weise haben Sie eine viel bessere Chance zu verstehen, was jede Funktion tut, da Sie den globalen Status nicht berücksichtigen müssen.

  Also anstatt zu verwenden Initialisieren Sie die Variablen in main() und übergeben Sie sie bei Bedarf als Argumente an Funktionen. In diesem Fall sehe ich nicht die Notwendigkeit, multiplier überhaupt außerhalb der Funktion zu verwenden. Lassen Sie es also einfach innerhalb der Funktion deklariert.

• sign sollte eine int und keine char sein

Algorithmus

• Im Moment implementieren Sie eine komplizierte und schwer zu befolgende Methode zum Konvertieren eines Zeichens in eine Zahl. Der einfache Weg ist, isdigit() die harte Arbeit für Sie erledigen zu lassen. Dies hilft Ihnen auch bei der Implementierung des DRY-Prinzip .

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

  Sehen Sie, wie zwei Schleifen fast identische Dinge tun? Hier ist, wie ich all das vereinfacht habe, indem ich isdigit() verwendet habe.

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

  Sie durchlaufen die Zeichen in der Zeichenfolge, solange sie Ziffern sind. Fügen Sie für jedes Zeichen den Zähler hinzu Sie behalten – der hinzuzufügende Wert ist der ganzzahlige Wert des Zeichens. Dies erfolgt durch Subtrahieren des ASCII-Werts von "0" vom ASCII-Wert der betreffenden Ziffer.

• Beachten Sie, dass dieser Code dies nicht tut Wenn Sie „89384798719061231“ übergeben (was nicht in eine int passt), ist das Ergebnis undefiniert. Das Update ist einfach genug. Verwenden Sie einfach eine long long int, um dem entgegenzuwirken. Wir werden immer noch Probleme mit extrem langen Zahlen haben, aber das zu beheben, damit die Funktion wie beabsichtigt funktioniert, ist etwas komplizierter.

Dokumentation

• Wohin gingen all Ihre Kommentare? Ein neuerer Entwickler hat einfach einen Teil Ihres Codes angesehen.

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

  Kommentare können wirklich dazu beitragen, dass andere Ihren Code besser verstehen. Gehen Sie jedoch nicht über Bord, Sie müssen ausgleichen, wie viel davon Sie können sie in Ihr Programm einfügen.

Syntax / Styling

• Dies sieht aus wie ein Tippfehler.

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

  Fügen Sie der Übersichtlichkeit ein Leerzeichen hinzu.

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

• Sie sollten nicht ändere deine char*, die du als Parameter in die Funktion akzeptierst. Deklarieren Sie den Parameter daher als konstant.

  int my_atoi(const char* pointer) 

• Verwenden Sie mehr Kurzzeichenoperatoren.

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

Endgültiger Code

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

Kommentare

• Sie sollten ‚ die Rückgabetypen nicht willkürlich ändern. atoi() gibt traditionell eine int zurück, daher sollte my_atoi() dies auch tun. Wenn Sie eine long long analysieren möchten, emulieren Sie strtoll().
• isdigit(*c) ist nicht für *c -Werte kleiner als 0 (außer EOF) definiert. Besser zu while (isdigit((unsigned char) (*c) ))
• Fehlende Ecke: Wenn my_atoi() Ergebnis LLONG_MIN, value += (int) (*c-'0'); ist ein vorzeichenbehafteter ganzzahliger Überlauf (UB), wenn versucht wird, LLONG_MAX + 1 zu bilden.
• Verwenden von isdigit ist überhaupt falsch, da ‚ keine verwandte Funktion numeric_value hat. Wenn Ihr Zeichensatz zwei Ziffernbereiche hat (0 bis 9 und 9 bis ٩), werden die indischen Zahlen daher falsch analysiert. Halten Sie sich aus Sicherheitsgründen einfach an '0' <= c && c <= '9'. Dadurch wird auch vermieden, dass das undefinierte Verhalten die Funktion ctype falsch verwendet.
• Sie haben einen wichtigen Punkt übersehen, als Sie den ASCII-Wert “ von ‚ 0 ‚ “ : dort ‚ s nichts, was besagt, dass der Host-Zeichensatz ASCII sein muss (nur dass 0..9 zusammenhängend sind). Aus diesem Grund schreiben Sie ‚ '0' anstelle einer codierungsspezifischen Codepunktnummer.

[Bearbeiten]

Mit Ausnahme des Verhaltens bei Fehlern ist atoi() gleichwertig zu (int)strtol(nptr, (char **)NULL, 10). strtol() akzeptiert führende Leerzeichen. OP „s my_atoi(char* pointer) tut dies nicht. Abhilfe:

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

Im Folgenden wird ein guter Umgang mit INT_MIN.

OTOH, die Übergabe von Werten außerhalb von [INT_MIN...INT_MAX] ist nicht durch die C-Spezifikation definiert, daher können einige Vereinfachungen vorgenommen werden Siehe weit unten.

Wenn eine Zeichenfolge INT_MIN darstellt (nehmen wir 32-Bit int) wie "-2147483648", Code läuft in einen int Überlauf, der versucht, 2147483648. Eine einfache Möglichkeit, dies zu lösen, besteht darin, den positiven Wert zu finden und ihn dann zu negieren und die negative Seite der Dinge zu berücksichtigen. Indem wir den Löwenanteil der Mathematik im Bereich INT_MIN bis 0 ausführen, vermeiden wir UB. Nachteil: Einige finden es schwieriger, diesem Ansatz zu folgen.

Bei einer breiteren Ganzzahl oder unsigned ist die Ganzzahlgröße von „text-“ nicht immer möglich. -> Integer „-Routine kann die maximale Größe sein. Genau genommen hat unsigned nicht immer einen größeren positiven Bereich als int. In jedem Fall kann die gesamte Mathematik mit der gewünschten Ganzzahlgröße mit Vorzeichen verarbeitet werden, ohne auf andere Typen zurückzugreifen.

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

Hinweise:

pointer%48 ist verwirrend. Was ist das Besondere an 48? Wenn Sie "0" meinen, verwenden Sie pointer % "0".

„string:“ 232-19 „. Was soll ich tun? dann tun? “ Es wird empfohlen, die Konvertierung bei „232“ zu stoppen und den Wert 232 zurückzugeben. Könnte iv id = „2d8eb24a65“>

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

Warum „Zeiger“ de-referenzieren? drei Mal? Einmal ist genug:

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

Kommentare

• Willkommen bei Code Review, Ihre erste Antwort sieht gut aus , genieße deinen Aufenthalt! Obwohl ich mich frage, ob es einen Unterschied im generierten Code macht.

, wenn Sie mit Rekursion einverstanden sind Dann könnte der Code auf einen Wert unter

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

verkürzt werden. Die Stapelerschöpfung könnte durch das -foptimize-sibling-calls Compiler-Flag gemindert werden wird sowohl von GCC- als auch von Clang-Compilern unterstützt.

Update:

Wie angegeben Die Implementierung von Roland Illig verarbeitet keine fehlerhaften Eingaben. Wenn es gewünscht wird, folgen Sie genau der atoi -Semantik , dann sollte der nächste Code fine _{vergiss nicht, Compile Options in Kommentaren} auf eins zu setzen.

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

Dies ist immer noch der Code von chux , in dem Schleifen durch Rekursion ersetzt werden.

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

Kommentare

• Testfall: buf = malloc(65536); buf[0] = '\0'; my_atoi(buf) wird wahrscheinlich abstürzen.
• Testfall: bufsize = 1 << 20; buf = malloc(bufsize); memset(buf, '0', bufsize); buf[bufsize - 1] = '\0'; my_atoi(buf) wird sehr lange dauern.

Für eine Übung in leetcode , schrieb folgendes 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; } }; 

Kommentare

• Willkommen bei Code Review! Sie haben eine alternative Lösung vorgestellt, aber ‚ hat den Code nicht überprüft. Bitte erläutern Sie Ihre Überlegungen (wie Ihre Lösung funktioniert und warum sie besser als das Original ist), damit der Autor und andere Leser aus Ihrem Denkprozess lernen können.
• Der Code verwendet eine Methode, wobei checkMin, wo nein Die direkte Multiplikation wird durchgeführt, bis das Ergebnis validiert ist. größer als INT_MIN sein.