Asiat, joita voit parantaa

Muuttujat / Alustus

• Missä ilmoitat multiplier? Oletan, että koska sitä ei ilmoiteta menetelmän sisällä, se ilmoitetaan globaalina muuttujana. Yritä välttää globaaleja muuttujia.

  Globaalien muuttujien ongelmana on, että koska jokaisella toiminnolla on pääsy näihin, on yhä vaikeampaa selvittää, mitkä toiminnot todella lukevat ja kirjoittavat näitä muuttujia.

  Jotta voisit ymmärtää sovelluksen toiminnan, sinun on melko paljon otettava huomioon kaikki toiminnot, jotka muuttavat globaalia tilaa. Se voidaan tehdä, mutta sovelluksen kasvaessa se vaikeutuu siihen pisteeseen, että se on käytännössä mahdotonta (tai ainakin täydellinen ajanhukkaa).

  Jos et luota globaaleihin muuttujiin, voit voi siirtää tilaa eri toimintojen välillä tarpeen mukaan. Tällä tavalla sinulla on paljon paremmat mahdollisuudet ymmärtää, mitä kukin toiminto tekee, koska sinun ei tarvitse ottaa huomioon globaalia tilaa.

  Joten käytön sijaan globaalit muuttujat, alustaa muuttujat ryhmässä main() ja välitä ne tarvittaessa argumentteina funktioille. Tässä tapauksessa en näe tarvetta käyttää multiplier -toimintoa lainkaan toiminnon ulkopuolelle, joten pidä se yksinkertaisesti ilmoitettuna funktion sisällä.

• sign tulee olla int eikä char .

Algoritmi

• Tällä hetkellä toteutat monimutkaisen ja vaikeasti seurattavan menetelmän muuntaa hahmo luvuksi. Helppo tapa on saada isdigit() tekemään kovaa työtä puolestasi. Tämä auttaa myös toteuttamaan KUIVAUS -periaate .

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

  Näetkö kuinka kaksi silmukkaa tekee melkein samanlaisia asioita? Näin yksinkertaistin kaiken käyttämällä isdigit().

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

  Voit selata merkkijonon merkkejä niin kauan kuin ne ovat numeroita. Lisää jokaiselle merkille laskuriin pidät – lisättävä arvo on merkin kokonaisluku. Tämä tehdään vähentämällä "0": n ASCII-arvo kyseisen numeron ascii-arvosta.

• Huomaa, että tämä koodi ei ”t käsittele ylivuotoa. Jos ohitat” 89384798719061231 ”(joka ei sovi int -tulokseen), tulos on määrittelemätön. Korjaus on tarpeeksi yksinkertainen, käytä vain long long int -ohjelmaa lieventääkseen sitä. Meillä on vielä ongelmia erittäin pitkien numeroiden kanssa, mutta sen korjaaminen, jotta funktio toimii tarkoitetulla tavalla, on hieman monimutkaisempi.

Dokumentaatio

• Minne kaikki kommenttisi menivät? Uudempi kehittäjä yksinkertaisesti haastaa osan koodistasi.

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

  Kommentit voivat todella auttaa paljon auttamaan muita ymmärtämään koodiasi. Älä kuitenkaan mene yli laidan heidän kanssaan, sinun on tasapainotettava, kuinka paljon ne sisällytettäväksi ohjelmaan.

Syntaksi / muotoilu

• Tämä näyttää kirjoitusvirheeltä.

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

  Lisää tilaa selkeyden lisäämiseksi.

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

• ei muokkaa char*, jonka hyväksyt parametrina funktioon. Siksi julista parametri vakiona.

  int my_atoi(const char* pointer) 

• Käytä enemmän lyhytoperaattoreita.

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

Lopullinen koodi

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

Kommentit

• Palautustyyppejä ei pidä ’ muuttaa mielivaltaisesti. atoi() palauttaa perinteisesti int, joten myös my_atoi() pitäisi olla. Jos haluat jäsentää long long, jäljittele strtoll().
• isdigit(*c) ei määritellä *c -arvoille, jotka ovat pienempiä kuin 0 (muut kuin EOF). Parempi while (isdigit((unsigned char) (*c) ))
• puuttuva kulma: Kun my_atoi() tuloksen tulisi olla LLONG_MIN, value += (int) (*c-'0'); on allekirjoitettu kokonaisluvun ylivuoto (UB), kun se yrittää muodostaa LLONG_MAX + 1.
• isdigit on lainkaan väärä, koska sillä ei ole ’ siihen liittyvää toimintoa numeric_value. Siksi, jos merkistösi sisältää kaksi numeroväliä (0–9 ja ٠ – ٩), indikaattorit numeroidaan väärin. Pysy vain '0' <= c && c <= '9' -kohdassa turvassa. Tämä estää myös määrittelemättömän käyttäytymisen käyttämästä ctype-toimintoa väärin.
• Ohitit tärkeän kohdan kirjoittaessasi ” ASCII-arvon ’ 0 ’ ” : siellä ’ Mikään, joka sanoo, että isäntämerkkijoukon on oltava ASCII (vain että 0..9 ovat vierekkäisiä). Siksi ’ syy, miksi kirjoitat '0' kuin koodauskohtaisen koodipisteen numeron.

[Muokkaa]

Lukuun ottamatta virhekäyttäytymistä, atoi() on vastaava kohtaan (int)strtol(nptr, (char **)NULL, 10). strtol() hyväksyy etätilan. OP ”s my_atoi(char* pointer) ei. Korjaus:

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

Alla on hyvä tapa käsitellä INT_MIN.

OTOH, arvojen luovuttamista [INT_MIN...INT_MAX] ulkopuolelle ei ole määritelty C-määrityksessä, joten joitain yksinkertaistuksia voidaan oli. Katso kaukana alla.

Kun merkkijono edustaa INT_MIN, (olettakaamme ”32-bittisen int), kuten "-2147483648", koodi törmää int ylivuotoon yrittäen laskea 2147483648. Yksinkertainen tapa ratkaista tämä on pikemminkin kuin löytää positiivinen arvo ja sitten hylätä se, omaksua asioiden negatiivinen puoli . Kun teemme leijonan matematiikasta INT_MIN – 0 -alueelle, vältämme UB: n. Alaspäin: Joidenkin mielestä tätä lähestymistapaa on haastavampaa seurata.

Laajemman kokonaisluvun tai unsigned siirtyminen ei aina ole mahdollista ”text- -> kokonaisluku ”rutiini voi olla enimmäiskoko. Tarkkaan ottaen unsigned ei ole aina laajempaa positiivista aluetta kuin int. Joka tapauksessa kaikki matematiikat voidaan hoitaa halutulla allekirjoitetulla kokonaislukukoolla turvautumatta muihin tyyppeihin.

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

Huomautuksia:

pointer%48 on hämmentävää. Mikä on erityistä 48: ssa? Jos tarkoitat "0", käytä pointer % "0".

”merkkijono:” 232-19 ”. Mitä minun pitäisi tehdä tehdä sitten? ” Suosittele muunnoksen lopettamista kohdassa ”232” ja arvon 232 palauttamista. Voisi asettaa errno, mutta tyypillinen atoi() funktio ei tee liikaa virheenkäsittelyä.

Ylivuotoa käytettäessä errno -asetus voi tapahtua, mutta taas tyypillinen atoi() -toiminto ei tee liikaa virheiden käsittelyä. Ehdota yksinkertaista palautusta INT_MAX tai INT_MIN.

Jos haluat paremman virhekäsittelyn, vaihda jotain seuraavista ja aseta virhetila.

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

tai sijainti missä asiat päättyivät. Jos tämä on hyvä, ne päättyivät kohtaan "\0".

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

[Muokkaa] Yksinkertaistettu: poistettu alueen ulkopuolella oleva havaitseminen, koska C spec sallii sen. ”Jos tuloksen arvoa ei voida esittää, käyttäytymistä ei ole määritelty.

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

Kommentit

• INT_MIN/10 ja INT_MIN%10 vaativat C99-käyttäytymistä.

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

Miksi ”osoittimen” viittaus poistetaan kolme kertaa? Yksi kerta riittää:

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

kommentit

• Tervetuloa Code Review -sivustoon, ensimmäinen vastauksesi näyttää hyvältä , Nauti vierailustasi! Vaikka ihmettelen, onko sillä eroa luotavassa koodissa.

jos olet kunnossa rekursiolla sitten koodi voidaan lyhentää yhdeksi alla olevasta.

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

Pinon ehtymistä voidaan vähentää -foptimize-sibling-calls kääntäjän lipulla, joka on sekä GCC- että Clang-kääntäjät tukevat.

Päivitys:

Kuten on mainittu Roland Illig in toteutus ei käsittele vääristynyttä syötettä. Jos sitä halutaan sulkea, seuraa atoi semantiikkaa , seuraavan koodin tulee olla fine _{älä unohda asettaa Compile Options yhdeksi kommenteissa} .

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

Tämä on edelleen chux -koodi, jossa silmukat korvataan rekursiolla

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

Kommentit

• Testitapaus: buf = malloc(65536); buf[0] = '\0'; my_atoi(buf) todennäköisesti kaatuu.
• Testitapaus: bufsize = 1 << 20; buf = malloc(bufsize); memset(buf, '0', bufsize); buf[bufsize - 1] = '\0'; my_atoi(buf) kestää hyvin kauan.

Harjoitukseen leetcode , kirjoitti seuraavan impl: atoi cpp-koodi

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

Kommentit

• Tervetuloa Code Review -sovellukseen! Olet esittänyt vaihtoehtoisen ratkaisun, mutta ’ ei ole tarkistanut koodia. Selitä perustelut (miten ratkaisusi toimii ja miksi se on alkuperäistä parempi), jotta kirjoittaja ja muut lukijat voivat oppia ajatusprosessistasi.
• koodi käyttää menetelmää, jossa checkMin, missä ei suora kertolasku suoritetaan, kunnes tulos on vahvistettu. olla suurempi kuin INT_MIN.