Fejléc

C-ben az összes enum név ugyanazt a névteret osztja meg egymással ( és olyan dolgokkal, mint a változó nevek). Ezért jó ötlet megpróbálni csökkenteni az ütközés kockázatát.

Az enum json_value_type nevek , ami elég általános. Néhány másik könyvtár megpróbálhatja ugyanazt a nevet használni. Azt javaslom, hogy változtassa meg ezt az előtagot mondjuk JSON_ -re.

Úgy tűnik, hogy nem használja a TYPE_KEY bármire. Csak távolítsa el.

Végrehajtás

Ahogy Roland Illig megjegyzi , a és isspace() a skip_whitespace() függvényben a unsigned char kerülje a jelkiterjesztést.

Alternatív megoldásként, és szorosabban követve a JSON specifikációt , egyszerűen átírhatja ezt a függvényt:

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

Sok static segítő funkciója nem triviális dolgok kombinációját végzi, és hiányzik azokból a megjegyzésekből, amelyek elmagyaráznák, mit csináld. Az egyes függvények előtt egy vagy két megjegyzéssor sokat segíthet az olvashatóságban.

Különösen a has_char() függvényed csinál egy csomó különböző dolgot:

1. Átugorja a szóközt.
2. Ellenőrzi, hogy van-e egy adott karakter a bemenetben.
3. Ha megtalálja a karaktert, automatikusan kihagyja azt.

A függvény neve nyilvánvalóan csak a 2. helyet foglalja magában; a többi váratlan mellékhatás, és legalább egyértelműen dokumentálni kell őket.

Valójában úgy tűnik számomra, hogy jobb lehet eltávolítani a skip_whitespace() innen: has_char(), és hagyja, hogy a hívó kifejezetten átugorja a szóközt, mielőtt szükség esetén felhívná. Sok esetben a kódod már megteszi ezt, feleslegessé téve az átugrást.

Emellett, hogy a 3. effektus kevésbé meglepő legyen az olvasó számára, célszerű lehet ezt a függvényt valamivel többre átnevezni. aktív, például mondjuk: read_char().

A json_parse_object() végén a következők vannak:

 return success; return 1; } 

Biztos, hogy ez felesleges. Csak szabaduljon meg a return 1; -től.

Továbbá, Úgy tűnik, hogy az általános json_parse_value() függvényt használja az objektumkulcsok elemzéséhez, és ne tesztelje, hogy azok karakterláncokat tartalmaznak-e. Ez lehetővé teszi néhány érvénytelen JSON számára az elemzőn való átjutást. Azt javaslom, hogy adjon hozzá egy kifejezett típusellenőrzést, vagy ossza szét a karakterlánc-elemző kódot egy külön függvénybe (az alábbiakban leírtak szerint), és hívja meg közvetlenül a json_parse_object() címről.

A json_parse_array() tetején van:

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

Ugyanúgy írhatná át, mint a következőben teszed: json_parse_object():

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

(Csak, tudod, én még mindig úgy gondolom, hogy a név read_char() jobb lenne.)

Valamilyen oknál fogva a json_parse_array() is azt várja, hogy a hívó inicializálja a parent struct, míg a json_parse_object() automatikusan megteszi. AFAICT nincs oka az inkonzisztenciára, ezért mindkét funkciót működtetnie kell és valószínűleg csak ugyanúgy.

A json_is_literal() függvényed nincs megjelölve static névvel, annak ellenére, hogy nem “t” megjelennek a fejlécben. Mint is_char(), én “d al inkább nevezze át valami aktívabbra, például json_read_literal() vagy csak read_literal(), hogy egyértelműbb legyen, hogy automatikusan a kurzort mozgatja egy sikeres egyezés.

(Vegye figyelembe azt is, hogy ahogy írták, ez a függvény nem nem ellenőrzi, hogy a bemenetben szereplő literál valóban ott fejeződik-e be, ahol kellene. Például sikeresen megegyezik a nullnullnull bemenettel a null bemenettel.Nem gondolom, hogy ez tényleges hiba, mivel a JSON-ban az egyetlen érvényes literál a true, false és null, amelyek egyike sem egymás előtagja, és mivel két literál nem jelenhet meg egymás után az érvényes JSON-ban anélkül, hogy közöttük lenne más jelző. De mindenképpen érdemes megjegyezni egy megjegyzésben.)

Érdemes kifejezetten megjelölni néhány statikus segítő funkcióját inline tippet adni a fordítónak, hogy meg kell próbálnia egyesíteni őket a hívó kódban. Azt javaslom, hogy tegye ezt legalább skip_whitespace(), has_char() és json_is_literal().

Mivel a json_value_to_X() hozzáférési függvények nem másból állnak, csak egy és a mutató elhivatottsága esetén fontolóra kell vennie megvalósításaik áthelyezését a json.h fájlba és static inline jelölést. Ez lehetővé tenné a fordító számára, hogy még más .c fájlokban is beillessze őket a hívó kódba, és esetleg optimalizálja a assert() t, ha a hívó A kód egyébként is ellenőrzi a típust.

A fő json_parse() függvényben érdemes lehet kifejezetten ellenőrizni, hogy a bemenet a gyökérérték elemzése után.

Karakterlánc-elemzés

A (z) json_parse_value() karakterlánc-elemző kód megszakadt, mivel nem “t” fogantyú visszavágás menekül. Például nem sikerül a következő érvényes JSON bemeneten:

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

Ezt hozzáadhatja tesztesetként.

Ön azt is tesztelnie kell, hogy a kódod megfelelően kezeli-e a visszavágó menekülési szekvenciákat, például \b, \f, \n, \r, \t, \/ és különösen \\ és \unnnn. Itt van még néhány teszteset azok számára:

"\"\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" 

Mivel a JSON karaktersorozatok tetszőleges Unicode karaktereket tartalmazhatnak, el kell döntenie, hogyan kell azokat kezelni. Valószínűleg a legegyszerűbb választás az lenne, ha a bemenetét és a kimenetét UTF-8-ban (vagy esetleg WTF-8 ) deklarálja, és a \unnnn az UTF-8 bájt szekvenciákba menekül (és adott esetben fordítva). Ne feledje, hogy mivel nullával végződött karakterláncokat használ, akkor inkább a \u0000 kódot dekódolja a túl hosszú kódolású "\xC0\x80" null bájt helyett.

A json_parse_value() fő funkció olvashatósága érdekében Erősen ajánlom, hogy ossza szét a karakterlánc-elemző kódot egy külön segítő függvényre. Különösen azért, mert ha a visszavágó menekülést helyesen kezeli, az jelentősen megnehezíti.

Az egyik bonyodalom az, hogy nem fogod tudni, hogy meddig A karakterlánc addig lesz, amíg nem elemezte. Ennek kezelésére az lenne a lehetőség, ha dinamikusan növelné a kiosztott kimeneti karakterláncot a következővel: realloc(), pl. így:

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

Alternatív megoldás lehet két elemzési lépés futtatása a bemeneten: az egyik csak a kimeneti karakterlánc hosszának meghatározása, a másik pedig a tényleges dekódolása. Ez a legkönnyebben megvalósítható valami ilyesmi:

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

ahol a segítő függvény:

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

legfeljebb JSON karakterláncot elemez *length bájt a new_string bájtba, és az elemzett karakterlánc tényleges hosszát a *length beírja, vagy ha new_string == NULL, akkor csak a karakterlánc hosszát határozza meg, anélkül, hogy a dekódolt kimenetet ténylegesen bárhol tárolná.

Számelemzés

A jelenlegi json_parse_value() megvalósítás a számokat alapértelmezettként kezeli, és egyszerűen betáplál mindent, ami nem létezik ", [, {, n, t vagy f be a C standard könyvtár függvényébe strtod().

Mivel strtod() elfogadja a egy érvényes készlet JSON-szám literálból, ennek működnie kell, de a kód néha változik az érvénytelen JSON-t érvényesnek tekintette. Például a kód elfogadja a következőt: +nan, -nan, +inf és -inf érvényes számként, és elfogadja a hexadecimális jelölést is, például 0xABC123. Továbbá, amint a fent hivatkozott strtod() dokumentáció megjegyzi:

A “C” szabványtól eltérő nyelven vagy “POSIX” területi beállításokkal, ez a funkció felismerhet további területi beállításoktól függő szintaxist.

Ha szigorúbb akar lenni, érdemes kifejezetten érvényesíteni bármit, ami számnak tűnik, a JSON nyelvtan , mielőtt átadná a strtod() címzettnek.

Ne feledje, hogy a strtod() beállíthatja a errno pl ha a bemeneti szám kívül esik egy double tartományon. Valószínűleg ellenőriznie kell ezt.

Tesztelés

Nem néztem meg részletesen a tesztjeit, de nagyon jó látni, hogy megvannak (még akkor is, ha megjegyeztük a lefedettségük javulhat).

Személy szerint azonban inkább a teszteket külön forrásfájlba helyezem át a megvalósításból. Ennek vannak előnyei és hátrányai is:

• A fő hátrány az, hogy már nem lehet közvetlenül tesztelni a statikus segítő funkciókat. Tekintettel azonban arra, hogy nyilvános API-ja tiszta és átfogónak tűnik, és nem szenved olyan „rejtett állapotú” problémáktól, amelyek bonyolítanák a tesztelést, képesnek kell lennie arra, hogy jó tesztfedettséget érjen el még az API-n keresztül is.
• A fő előny (a megvalósítás és a tesztkód tiszta elkülönítése mellett) az, hogy a tesztjei automatikusan tesztelik a nyilvános API-t. Különösen az json.h fejléccel kapcsolatos problémák jelennek meg A tesztek elvégzése az API-n keresztül segít abban is, hogy az API valóban kellően teljes és rugalmas legyen az általános használatra.

Ha valóban szeretné közvetlenül tesztelni a statikus funkcióit, mindig hozzáadhat egy előfeldolgozó jelzőt, amely opcionálisan kiállítja őket tesztelésre, akár egyszerű csomagolók segítségével, akár csak az static kulcsszó eltávolításával a definícióikból.

Ps. I észrevette, hogy json_test_value_number() tesztje sikertelen számomra (GCC 5.4.0, i386 arch), feltehetően mert a 23.4 szám nem pontosan ábrázolható lebegőpontban. Ha 23,5-re változtatja, a teszt sikeres lesz.

Megjegyzések

• Nagyszerű munka, köszönöm, nyilvánvalóan nem tettem ‘ t ellenőrizze a szabványt, amennyire kellett volna. Bár szándékosan figyelmen kívül hagytam az utf-8-at (valószínűleg ezt meg kellett volna említenem.
• OK, elég korrekt. Valószínűleg még mindig dekódolnia kell az \u00XX, mivel egyes kódolók úgy dönthetnek, hogy akár az ASCII karakterekhez is felhasználják őket. (Ezenkívül javaslatot tettem egyes funkciók fent inline jelzésére, mivel ezt korábban elfelejtettem megtenni .)
• Teljesen hiányoltam a szabványnak azt a részét, igen, a menekülési szekvenciákat helyesen kell értelmezni.

Ez egyáltalán nem teljes körű áttekintés, de megosztok néhány olyan dolgot, amelyre a kód olvasása közben figyeltem fel.

Megjegyzések

Míg a megjegyzések biztosan szépek, néhány beágyazott megjegyzésed csak zajt ad a kódba.

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

Először is, a megjegyzés egy sorral túl korai. Másodszor, leolvasható, hogy a szóköz elfogyasztja a függvényt – a név tökéletesen leírja, th Nincs szükség további megjegyzésre.

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

Ez a megjegyzés ismét megismétli azt, amit maga a kód mond.

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

Ezek a megjegyzések nem igazán haszontalanok, mivel dokumentálják, hogy az egyes értékek mit képviselnek. De miért csak a TYPE_OBJECT és a TYPE_ARRAY esetén – miért nem minden értéknél? Személy szerint csak egy linket tettem a json.org linkre közvetlenül azelőtt enum. Típusai analógak amik ott vannak, akkor csak azt kell dokumentálnia, aminek állítólag TYPE_KEY kell lennie. Ez a következő pontra vezet …

TYPE_KEY

Ha megnézi a json.org webhelyet, láthatja, hogy egy objektum áll a tagok listája, amelyek viszont egy string ből és egy érték ből állnak. Ez azt jelenti, hogy nem igazán kell a TYPE_KEY! Csak adj hozzá egy új struktúrát a tagok számára , amely egy TYPE_STRING értékből és egy másik bármilyen típusú json értékből áll, és máris indulhatsz. Most pl. egy szám kulcsa az értéknek, ami nem megengedett. Az objektumokkal kapcsolatos logikák egy részét is szebbé tenné, például a ciklusnál:

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

Ironikus módon ennek a ciklusnak a lépése valóban használhat megjegyzést (miért += 2?), De hiányzik belőle.

Vegyes

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

Miért ne csak return 0;?

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

és

if (success) ++(*cursor); 

és

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

és még néhány ilyen. Én nem különösen szereti, ha a feltételeket és az állításokat ugyanarra a sorra helyezi, különösen azért, mert ezt nem következetesen csinálja.”Rendben van, ha ezt teszem a vezérlő áramlás érdekében, például if (!something) return;, de ez még mindig” meh “. Jobb, ha jól csinálod, és tedd az utasítást egy új sorba.

Ezenkívül azt tapasztalom, hogy a kódod használhat még néhány üres sort a régiók elválasztására, vagy bármilyen más néven. Például:

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; 

Egy üres sor választja el a setup-and-parse-cuccokat a check-and-return dolgokból, de megteheti jobb.

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; 

Ezt sokkal tisztábbnak találom. Van egy blokk az értékek beállításához, egy blokk az elemzéshez, egy blokk az elhelyezéshez a vektorba, egy blokk a szóköz átugrására és egy blokk az aktuális művelet véglegesítésére. Az utolsó üres sor skip_whitespace(cursor); és if ... között vitatható , de én ezt jobban szeretem.

Ezen kívül a kódját könnyen olvashatónak és érthetőnek találtam. Megfelelően ellenőrzi az esetleges hibákat és ésszerű elnevezést használ. Ami az idiomatikát illeti, abból, amit említettem, nincs semmi, amit szokatlannak vagy nem idomatikusnak jelölnék.

A ctype.h függvényeket nem szabad meghívni , mivel ez meghatározhatatlan viselkedést idézhet elő. Jó magyarázatot a NetBSD dokumentációban talál.