Otsikko

C: ssä kaikki enum nimet jakavat saman nimiavaruuden keskenään ( ja esimerkiksi muuttujien nimien kanssa). Siksi on hyvä yrittää vähentää törmäysriskiä.

enum json_value_type -nimilläsi on etuliite TYPE_, joka on melko yleinen. Jotkut muut kirjastot saattavat yrittää käyttää samaa nimeä. Ehdotan, että muutat kyseisen etuliitteen sanaksi JSON_.

Et myöskään näytä käyttävän TYPE_KEY mihin tahansa. Poista se vain.

Toteutus

Kuten Roland Illig toteaa , ja isspace() skip_whitespace() -funktiossa tulisi lähettää ryhmään unsigned char vältä merkkilaajennusta.

Vaihtoehtoisesti ja seuraamalla tarkemmin JSON-määritystä , voit kirjoittaa tämän funktion uudestaan seuraavasti:

static void skip_whitespace(const char** cursor) { while (**cursor == "\t" || **cursor == "\r" || **cursor == "\n" || **cursor == " ") ++(*cursor); } 

Monet static -aputoiminnot tekevät ei-triviaalia asioiden yhdistelmiä, eikä niissä ole kommentteja, jotka selittäisivät mitä ne tehdä. Yksi tai kaksi kommenttiriviä ennen kutakin toimintoa voisi auttaa paljon luettavuutta.

Erityisesti has_char() -funktiosi tekee joukon erilaisia asioita:

1. Se ohittaa välilyönnin.
2. Se tarkistaa tietyn merkin esiintymisen syötteessä.
3. Jos merkki löytyy, se ohittaa sen automaattisesti.

Funktion nimi tarkoittaa ilmeisesti vain # 2; muut ovat odottamattomia sivuvaikutuksia, ja ne on ainakin dokumentoitava selkeästi.

Oikeastaan minusta tuntuu, että on parempi poistaa puhelu osoitteeseen skip_whitespace() osoitteesta has_char() ja anna soittajan vain ohittaa välilyönti ennen kuin soitat siihen tarvittaessa. Monissa tapauksissa koodisi tekee niin jo, jolloin kaksoiskappale ohitetaan tarpeettomaksi.

Jotta vaikutus # 3 olisi vähemmän yllättävä lukijalle, voi olla hyvä idea nimetä funktio uudeksi nimeksi hieman enemmän aktiivinen, kuten esimerkiksi read_char().

Kohteen json_parse_object() lopussa sinulla on:

 return success; return 1; } 

Varmasti se on tarpeeton. Poista vain return 1;.

näyttää siltä, että käytät yleistä json_parse_value() -funktiota objektiavain jäsentämiseen ja älä testaa varmistaaksesi, että ne merkitsevät merkkijonoja. Tämän avulla jotkut virheelliset JSON-tiedostot pääsevät jäsentäjän läpi. Ehdotan joko nimenomaisen tyyppitarkistuksen lisäämistä tai merkkijonon jäsentämiskoodin jakamista erilliseksi toiminnoksi (kuten alla on kuvattu) ja kutsumalla se suoraan osoitteesta json_parse_object().

json_parse_array() -kohdan yläosassa sinulla on:

if (**cursor == "]") { ++(*cursor); return success; } while (success) { 

Voit kirjoittaa sen samalla tavalla kuin teet kohdassa json_parse_object():

while (success && !has_char("]")) { 

(Vain, tiedätkö, mielestäni nimi read_char() olisi parempi.)

Myös jostain syystä json_parse_array() näyttää odottavan soittajan alustavan parent struct, kun taas json_parse_object() tekee sen automaattisesti. AFAICT ei ole mitään syytä epäjohdonmukaisuuteen, joten voit ja todennäköisesti pitäisi vain saada molemmat toiminnot toimimaan samalla tavalla.

json_is_literal() -toimintoa ei ole merkitty nimellä static, vaikka se ei olisikaan näkyvät otsikossa. Kuten is_char(), minä ”d al Joten mieluummin nimeä se aktiivisemmaksi, kuten json_read_literal() tai vain read_literal(), jotta on selvää, että se vie kohdistimen automaattisesti eteenpäin. onnistunut ottelu.

(Huomaa myös, että kirjoitettuna tämä toiminto ei tarkista, että syötteen kirjaimellinen kirjain loppuu oikeaan paikkaan. Esimerkiksi se sovittaa onnistuneesti syötteen nullnullnull ja null.En usko, että se on todellinen vika, koska JSON: n ainoat kelvolliset literaalit ovat true, false ja null, joista yksikään ei ole toistensa etuliite, ja koska kaksi literaalia ei voi esiintyä peräkkäin kelvollisessa JSON-tiedostossa ilman mitään muuta tunnusta niiden välissä. Mutta se on ehdottomasti ainakin huomionarvoinen kommentissa.)

Voit myös haluta nimenomaisesti merkitä joitain staattisia aputoimintojasi nimellä inline antaa kääntäjälle vihje, että sen pitäisi yrittää yhdistää ne kutsukoodiin. Ehdotan, että tekisit sen ainakin skip_whitespace(), has_char() ja json_is_literal().

Koska json_value_to_X() -lisätoiminnot eivät sisällä muuta kuin assert() ja osoittimen poikkeama, sinun tulisi myös harkita niiden toteutusten siirtämistä json.h -kohtaan ja merkitsemällä ne nimellä static inline. Tämän avulla kääntäjä voisi sisällyttää ne kutsukoodiin myös muissa .c -tiedostoissa, ja mahdollisesti optimoida assert(), jos kutsu code jo tarkistaa tyypin joka tapauksessa.

json_parse() -funktiossa kannattaa ehkä nimenomaisesti tarkistaa, että kentässä ei ole muuta kuin tyhjää tilaa syöttö juuriarvon jäsentämisen jälkeen.

Merkkijonon jäsentäminen

Merkkijonon jäsentämiskoodisi kohdassa json_parse_value() on rikki, koska se ei ”t kahva taaksepäin. Se epäonnistuu esimerkiksi seuraavassa kelvollisessa JSON-syötteessä:

"I say: \"Hello, World!\"" 

Voit lisätä sen testitapauksena.

Sinä tulisi myös testata, että koodisi käsittelee oikein muut taaksepäin osoittavat pakosarjat, kuten \b, \f, \n, \r, \t, \/ ja erityisesti \\ ja \unnnn. Tässä vielä muutama testitapaus niille:

"\"\b\f\n\r\t\/\\" "void main(void) {\r\n\tprintf(\"I say: \\\"Hello, World!\\\"\\n\");\r\n}" "\u0048\u0065\u006C\u006C\u006F\u002C\u0020\u0057\u006F\u0072\u006C\u0064\u0021" "\u3053\u3093\u306B\u3061\u306F\u4E16\u754C" 

Koska JSON-merkkijonot voivat sisältää mielivaltaisia Unicode-merkkejä, sinun on päätettävä, miten niitä käsitellään. Todennäköisesti yksinkertaisin vaihtoehto olisi julistaa tulosi ja lähtösi olevan UTF-8: ssa (tai ehkä WTF-8 ) ja muuntaa \unnnn pakenee UTF-8-tavusekvensseihin (ja mahdollisesti päinvastoin). Huomaa, että koska käytät nollapäätteisiä merkkijonoja, sinun kannattaa dekoodata \u0000 pitkäkoodaukseksi "\xC0\x80" nollatavun sijaan.

Jotta pääfunktio json_parse_value() pysyisi luettavissa, Suosittelen voimakkaasti merkkijonon jäsentämiskoodin jakamista erilliseksi auttajatoiminnoksi. Varsinkin kun sen tekeminen käänteisen viivan poistumiseksi oikein vaikeuttaa sitä huomattavasti.

Yksi komplikaatioista on, että et todellakaan tiedä kuinka kauan merkkijono on, kunnes olet jäsentänyt sen. Yksi tapa käsitellä sitä olisi kasvattaa varattua tulostusmerkkijonoa dynaamisesti realloc(): llä, esimerkiksi näin:

// resize output buffer *buffer to new_size bytes // return 1 on success, 0 on failure static int resize_buffer(char** buffer, size_t new_size) { char *new_buffer = realloc(*buffer, new_size); if (new_buffer) { *buffer = new_buffer; return 1; } else return 0; } // parse a JSON string value // expects the cursor to point after the initial double quote // return 1 on success, 0 on failure static int json_parse_string(const char** cursor, json_value* parent) { int success = 1; size_t length = 0, allocated = 8; // start with an 8-byte buffer char *new_string = malloc(allocated); if (!new_string) return 0; while (success && **cursor != """) { if (**cursor == "\0") { success = 0; // unterminated string } // we"re going to need at least one more byte of space while (success && length + 1 > allocated) { success = resize_buffer(&new_string, allocated *= 2); } if (!success) break; if (**cursor != "\\") { new_string[length++] = **cursor; // just copy normal bytes to output ++(*cursor); } else switch ((*cursor)[1]) { case "\\":new_string[length++] = "\\"; *cursor += 2; break; case "/": new_string[length++] = "/"; *cursor += 2; break; case """: new_string[length++] = """; *cursor += 2; break; case "b": new_string[length++] = "\b"; *cursor += 2; break; case "f": new_string[length++] = "\f"; *cursor += 2; break; case "n": new_string[length++] = "\n"; *cursor += 2; break; case "r": new_string[length++] = "\r"; *cursor += 2; break; case "t": new_string[length++] = "\t"; *cursor += 2; break; case "u": // TODO: handle Unicode escapes! (decode to UTF-8?) // note that this may require extending the buffer further default: success = 0; break; // invalid escape sequence } } success = success && resize_buffer(&new_string, length+1); if (!success) { free(new_string); return 0; } new_string[length] = "\0"; parent->type = TYPE_STRING; parent->value.string = new_string; ++(*cursor); // move cursor after final double quote return 1; } 

Vaihtoehtoinen ratkaisu olisi suorittaa kaksi jäsentelykulua syötteen yli: yksi vain määritetään lähtömerkkijonon pituus ja toinen dekoodata se. Tämä olisi helpoin tehdä jotain tällaista:

static int json_parse_string(const char** cursor, json_value* parent) { char *tmp_cursor = *cursor; size_t length = (size_t)-1; if (!json_string_helper(&tmp_cursor, &length, NULL)) return 0; char *new_string = malloc(length); if (!new_string) return 0; if (!json_string_helper(&tmp_cursor, &length, new_string)) { free(new_string); return 0; } parent->type = TYPE_STRING; parent->value.string = new_string; *cursor = tmp_cursor; return 1; } 

missä auttajafunktio:

static int json_parse_helper(const char** cursor, size_t* length, char* new_string) { // ... } 

jäsentää JSON-merkkijonon, jossa on enintään *length tavuina new_string ja kirjoittaa jäsennetyn merkkijonon todellisen pituuden osioon *length, tai jos new_string == NULL, vain määrittää merkkijonon pituuden tallentamatta dekoodattua lähtöä mihinkään.

Numeroiden jäsentäminen

Nykyinen json_parse_value() -toteutus käsittelee numeroita oletustapauksina ja syöttää yksinkertaisesti kaiken, mikä ei ole ", [, {, n, t tai f C-kirjastofunktioon strtod().

Koska strtod() hyväksyy superset voimassa olevista JSON-numeroista, tämä pitäisi toimia, mutta se voi tehdä koodistasi joskus ac hyväksyi virheellisen JSON: n kelvolliseksi. Koodisi hyväksyy esimerkiksi +nan, -nan, +inf ja -inf kelvollisina numeroina ja hyväksyy myös heksadesimaalimerkinnät, kuten 0xABC123. Lisäksi yllä linkitettynä strtod() -dokumentaationa huomautukset:

Muualla kuin normaalilla C-kielellä tai ”POSIX” -alueet, tämä toiminto voi tunnistaa ylimääräisen kielialueesta riippuvan syntaksin.

Jos haluat olla tiukempi, kannattaa ehkä vahvistaa kaikki, mikä näyttää numerolta, kohtaan JSON-kielioppi ennen sen välittämistä tiedostolle strtod().

Huomaa myös, että strtod() voi asettaa errno esim jos syötetty numero on double -alueen ulkopuolella. Sinun pitäisi todennäköisesti tarkistaa tämä.

Testaus

En ole tarkastellut testejäsi yksityiskohtaisesti, mutta on hienoa nähdä, että sinulla on ne (vaikka, kuten totesin yllä, niiden kattavuutta voitaisiin parantaa).

Henkilökohtaisesti haluaisin kuitenkin mieluummin siirtää testit ulos toteutuksesta erilliseen lähdetiedostoon. Tällä on sekä etuja että haittoja:

• Suurin haittapuoli on, että et voi enää suoraan testata staattisia auttajatoimintoja. Ottaen kuitenkin huomioon, että julkinen sovellusliittymäsi näyttää siistiltä ja kattavalta eikä kärsi mistään ”piilotetussa tilassa” olevista ongelmista, jotka vaikeuttavat testausta, sinun pitäisi pystyä saavuttamaan hyvä testipeitto jopa API: n kautta.
• Tärkein etu (toteutus- ja testauskoodien selkeän erotuksen lisäksi) on, että testisi testaavat automaattisesti julkisen sovellusliittymän. Erityisesti json.h -otsikon ongelmat näkyvät Testien tekeminen sovellusliittymän kautta auttaa myös varmistamaan, että sovellusliittymäsi on todella riittävän kattava ja joustava yleiseen käyttöön.

Jos haluat silti todella testata staattisia toimintojasi, voit aina lisätä esiprosessorin lipun, joka valinnaisesti altistaa ne testattavaksi joko yksinkertaisten kääreiden avulla tai vain poistamalla avainsana static määritelmistään.

Ps. I huomasin, että json_test_value_number() -testi epäonnistuu minulle (GCC 5.4.0, i386-kaari), oletettavasti koska luku 23.4 ei ole täsmällisesti edustettavissa liukuluvussa. Jos vaihdat sen arvoon 23.5, testi läpäisee.

Kommentit

• Hienoa työtä, kiitos, en tietenkään ’ t tarkista standardi niin paljon kuin minun pitäisi olla. Vaikka jätin utf-8: n huomiotta tahallaan (luultavasti minun olisi pitänyt mainita se.
• OK, riittävän oikeudenmukainen. Luultavasti silti sinun pitäisi ainakin dekoodata yksinkertaiset muodot \u00XX, koska jotkut kooderit saattavat halutessaan käyttää niitä jopa ASCII-merkkeihin. (Lisäsin myös ehdotuksen joidenkin toimintojesi merkitsemiseksi yllä oleviksi inline, koska unohdin tehdä sen aiemmin .)
• Kaipasin sitä standardin osaa täysin, kyllä pakosarjat on jäsennettävä oikein.

Tämä ei ole mitenkään täydellinen arvostelu, mutta jaan joitain asioita, jotka kiinnittivät huomiota koodia lukiessani.

Kommentit

Vaikka kommentit ovat varmasti hyviä, jotkut sisäisistä kommenteistasi lisäävät koodiin vain kohinaa.

// Eat whitespace int success = 0; skip_whitespace(cursor); 

Ensinnäkin kommentti on yksi rivi liian aikaisin. voidaan lukea, että välilyönti kulutetaan tarkastelemalla funktiota – nimi kuvaa sitä täydellisesti, th Ei tarvita lisäkommentteja.

case "\0": // If parse_value is called with the cursor at the end of the string // that"s a failure success = 0; break; 

Jälleen kerran tämä kommentti vain toistaa koodin itsensä sanan.

enum json_value_type { TYPE_NULL, TYPE_BOOL, TYPE_NUMBER, TYPE_OBJECT, // Is a vector with pairwise entries, key, value TYPE_ARRAY, // Is a vector, all entries are plain TYPE_STRING, TYPE_KEY }; 

Nämä kommentit eivät ole oikeastaan hyödyttömiä, koska ne dokumentoivat, mitä kukin arvo edustaa. Mutta miksi vain TYPE_OBJECT ja TYPE_ARRAY – miksi ei kaikille arvoille? Henkilökohtaisesti laitoin vain linkin json.org -palveluun juuri ennen sitä enum. Tyyppisi ovat analogisia tarvitset vain ne asiakirjat, joiden TYPE_KEY on tarkoitus olla. Tämä vie minut seuraavaan pisteeseen …

TYPE_KEY

Kun katsot kohdetta json.org , näet, että objekti koostuu luettelo jäsenistä , jotka puolestaan koostuvat merkkijonosta ja arvosta . Tämä tarkoittaa, että et todellakaan tarvitse TYPE_KEY! Lisää vain uusi rakenne jäsenille , joka koostuu TYPE_STRING -arvosta ja toisesta minkä tahansa tyyppisestä Json-arvosta, ja sinulla on hyvä mennä. Juuri nyt voit sinulla on esim. numero avaimenä arvolle, joka ei ole sallittu. Tekisi myös osasta olioihin liittyvää logiikkaa mukavampaa, kuten tämä silmukalle:

for (size_t i = 0; i < size; i += 2) 

Ironista kyllä, tämä vaihe silmukalle voisi tosiasiallisesti käyttää kommenttia (miksi += 2?), Mutta puuttuu.

Sekalaista

case "\0": // If parse_value is called with the cursor at the end of the string // that"s a failure success = 0; break; 

Miksi et vain return 0;?

while (iscntrl(**cursor) || isspace(**cursor)) ++(*cursor); 

ja

if (success) ++(*cursor); 

ja

if (has_char(cursor, "}")) break; else if (has_char(cursor, ",")) continue; 

ja muutama niistä. En ”ole erityisen kiinnostunut asettamaan ehto ja lauseke samalle riville, varsinkin kun et tee sitä jatkuvasti.Minulla on kunnossa tehdä tämä ohjausvirran vuoksi, kuten if (!something) return;, mutta se on silti ”meh”. Parempi tehdä se oikein ja laittaa lauseke uudelle riville.

Lisäksi huomaan, että koodisi voisi käyttää vielä tyhjiä rivejä erottaakseen ”alueet” tai mitä tahansa, mitä haluat kutsua heiksi Esimerkki:

json_value key = { .type = TYPE_NULL }; json_value value = { .type = TYPE_NULL }; success = json_parse_value(cursor, &key); success = success && has_char(cursor, ":"); success = success && json_parse_value(cursor, &value); if (success) { vector_push_back(&result.value.object, &key); vector_push_back(&result.value.object, &value); } else { json_free_value(&key); break; } skip_whitespace(cursor); if (has_char(cursor, "}")) break; else if (has_char(cursor, ",")) continue; else success = 0; 

On yksi tyhjä rivi, joka erottaa setup-and-parse-stuff -tarkistus- ja palautusasioista, mutta voit tehdä parempi.

json_value key = { .type = TYPE_NULL }; json_value value = { .type = TYPE_NULL }; success = json_parse_value(cursor, &key); success = success && has_char(cursor, ":"); success = success && json_parse_value(cursor, &value); if (success) { vector_push_back(&result.value.object, &key); vector_push_back(&result.value.object, &value); } else { json_free_value(&key); break; } skip_whitespace(cursor); if (has_char(cursor, "}")) break; else if (has_char(cursor, ",")) continue; else success = 0; 

Minusta tämä on paljon puhtaampaa. Sinulla on lohko arvojen määrittämiselle, lohko jäsentämiselle, lohko niiden asettamiselle vektoriin lohko välilyönnin ohittamiseksi ja lohko nykyisen toiminnon viimeistelemiseksi. Viimeinen tyhjä rivi skip_whitespace(cursor); ja if ... välillä on kiistanalainen , mutta pidän siitä mieluummin tällä tavalla.

Tämän lisäksi löysin koodisi helposti luettavaksi ja ymmärrettäväksi. Tarkistat virheet asianmukaisesti ja käytät järkevää nimeämistä. Idioottisuuden osalta siitä, mitä olen maininnut, ei ole mitään, mitä voisin merkitä epätavalliseksi tai epäidomaattiseksi.

Toimintoja lähteestä ctype.h ei saa kutsua argumenteilla, joiden tyyppi on char, koska se voi johtaa määrittelemättömään toimintaan. Katso hyvä selitys NetBSD-ohjeista .