Ecco un semplice parser JSON di discesa ricorsiva, non molte funzionalità extra, sebbene utilizzi la classe vettoriale espandibile recensita qui ( Vettore espandibile semplice in C ). Non ho implementato alcuna ottimizzazione, né uninterfaccia di accesso di livello superiore, è tutto piuttosto semplice. Inoltre, non esiste unesportazione JSON, ma solo limportazione.
Lintero codice sorgente è disponibile su github ( https://github.com/HarryDC/JsonParser ). CMake e codice di test inclusi, pubblicherò solo i file del parser qui.
Il mio interesse principale sarebbe se ci fossero modi più idiomatici per scrivere cose in C. Ma ovviamente anche qualsiasi altro input è sempre apprezzato.
Intestazione
#ifndef HS_JSON_H #define HS_JSON_H #include "vector.h" 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 }; typedef struct { int type; union { int boolean; double number; char* string; char* key; vector array; vector object; } value; } json_value; // Parse string into structure of json elements and values // return 1 if successful. int json_parse(const char* input, json_value* root); // Free the structure and all the allocated values void json_free_value(json_value* val); // Convert value to string if possible, asserts if not char* json_value_to_string(json_value* value); // Convert value to double if possible asserts if not double json_value_to_double(json_value* value); // Convert value to bool if possible asserts if not int json_value_to_bool(json_value* value); // Convert value to vector if it"s an array asserts if not vector* json_value_to_array(json_value* value); // Convert value to vector if it"s an object, asserts if not vector* json_value_to_object(json_value* value); // Fetch the value with given index from root, asserts if root is not array json_value* json_value_at(const json_value* root, size_t index); // Fetche the value with the given key from root, asserts if root is not object json_value * json_value_with_key(const json_value * root, const char * key); #ifdef BUILD_TEST void json_test_all(void); #endif #endif Implementazione
#include "json.h" #include <assert.h> #include <ctype.h> #include <stddef.h> #include <stdlib.h> #include <string.h> static int json_parse_value(const char ** cursor, json_value * parent); static void skip_whitespace(const char** cursor) { if (**cursor == "\0") return; while (iscntrl(**cursor) || isspace(**cursor)) ++(*cursor); } static int has_char(const char** cursor, char character) { skip_whitespace(cursor); int success = **cursor == character; if (success) ++(*cursor); return success; } static int json_parse_object(const char** cursor, json_value* parent) { json_value result = { .type = TYPE_OBJECT }; vector_init(&result.value.object, sizeof(json_value)); int success = 1; while (success && !has_char(cursor, "}")) { 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; } if (success) { *parent = result; } else { json_free_value(&result); } return success; return 1; } static int json_parse_array(const char** cursor, json_value* parent) { int success = 1; if (**cursor == "]") { ++(*cursor); return success; } while (success) { json_value new_value = { .type = TYPE_NULL }; success = json_parse_value(cursor, &new_value); if (!success) break; skip_whitespace(cursor); vector_push_back(&parent->value.array, &new_value); skip_whitespace(cursor); if (has_char(cursor, "]")) break; else if (has_char(cursor, ",")) continue; else success = 0; } return success; } void json_free_value(json_value* val) { if (!val) return; switch (val->type) { case TYPE_STRING: free(val->value.string); val->value.string = NULL; break; case TYPE_ARRAY: case TYPE_OBJECT: vector_foreach(&(val->value.array), (void(*)(void*))json_free_value); vector_free(&(val->value.array)); break; } val->type = TYPE_NULL; } int json_is_literal(const char** cursor, const char* literal) { size_t cnt = strlen(literal); if (strncmp(*cursor, literal, cnt) == 0) { *cursor += cnt; return 1; } return 0; } static int json_parse_value(const char** cursor, json_value* parent) { // Eat whitespace int success = 0; skip_whitespace(cursor); switch (**cursor) { case "\0": // If parse_value is called with the cursor at the end of the string // that"s a failure success = 0; break; case """: ++*cursor; const char* start = *cursor; char* end = strchr(*cursor, """); if (end) { size_t len = end - start; char* new_string = malloc((len + 1) * sizeof(char)); memcpy(new_string, start, len); new_string[len] = "\0"; assert(len == strlen(new_string)); parent->type = TYPE_STRING; parent->value.string = new_string; *cursor = end + 1; success = 1; } break; case "{": ++(*cursor); skip_whitespace(cursor); success = json_parse_object(cursor, parent); break; case "[": parent->type = TYPE_ARRAY; vector_init(&parent->value.array, sizeof(json_value)); ++(*cursor); skip_whitespace(cursor); success = json_parse_array(cursor, parent); if (!success) { vector_free(&parent->value.array); } break; case "t": { success = json_is_literal(cursor, "true"); if (success) { parent->type = TYPE_BOOL; parent->value.boolean = 1; } break; } case "f": { success = json_is_literal(cursor, "false"); if (success) { parent->type = TYPE_BOOL; parent->value.boolean = 0; } break; } case "n": success = json_is_literal(cursor, "null"); break; default: { char* end; double number = strtod(*cursor, &end); if (*cursor != end) { parent->type = TYPE_NUMBER; parent->value.number = number; *cursor = end; success = 1; } } } return success; } int json_parse(const char* input, json_value* result) { return json_parse_value(&input, result); } char* json_value_to_string(json_value* value) { assert(value->type == TYPE_STRING); return (char *)value->value.string; } double json_value_to_double(json_value* value) { assert(value->type == TYPE_NUMBER); return value->value.number; } int json_value_to_bool(json_value* value) { assert(value->type == TYPE_BOOL); return value->value.boolean; } vector* json_value_to_array(json_value* value) { assert(value->type == TYPE_ARRAY); return &value->value.array; } vector* json_value_to_object(json_value* value) { assert(value->type == TYPE_OBJECT); return &value->value.object; } json_value* json_value_at(const json_value* root, size_t index) { assert(root->type == TYPE_ARRAY); if (root->value.array.size < index) { return vector_get_checked(&root->value.array,index); } else { return NULL; } } json_value* json_value_with_key(const json_value* root, const char* key) { assert(root->type == TYPE_OBJECT); json_value* data = (json_value*)root->value.object.data; size_t size = root->value.object.size; for (size_t i = 0; i < size; i += 2) { if (strcmp(data[i].value.string, key) == 0) { return &data[i + 1]; } } return NULL; } #ifdef BUILD_TEST #include <stdio.h> void json_test_value_string(void) { printf("json_parse_value_string: "); // Normal parse, skip whitespace const char* string = " \n\t\"Hello World!\""; json_value result = { .type = TYPE_NULL }; assert(json_parse_value(&string, &result)); assert(result.type == TYPE_STRING); assert(result.value.string != NULL); assert(strlen(result.value.string) == 12); assert(strcmp("Hello World!", result.value.string) == 0); json_free_value(&result); // Empty string string = "\"\""; json_parse_value(&string, &result); assert(result.type == TYPE_STRING); assert(result.value.string != NULL); assert(strlen(result.value.string) == 0); json_free_value(&result); printf(" OK\n"); } void json_test_value_number(void) { printf("json_test_value_number: "); const char* string = " 23.4"; json_value result = { .type = TYPE_NULL }; assert(json_parse_value(&string, &result)); assert(result.type == TYPE_NUMBER); assert(result.value.number == 23.4); json_free_value(&result); printf(" OK\n"); } void json_test_value_invalid(void) { printf("json_test_value_invalid: "); { // not a valid value const char* string = "xxx"; json_value result = { .type = TYPE_NULL }; assert(!json_parse_value(&string, &result)); assert(result.type == TYPE_NULL); json_free_value(&result); } { // parse_value at end should fail const char* string = ""; json_value result = { .type = TYPE_NULL }; assert(!json_parse_value(&string, &result)); assert(result.type == TYPE_NULL); json_free_value(&result); } printf(" OK\n"); } void json_test_value_array(void) { printf("json_test_value_array: "); { // Empty Array const char* string = "[]"; json_value result = { .type = TYPE_NULL }; assert(result.value.array.data == NULL); assert(json_parse_value(&string, &result)); assert(result.type = TYPE_ARRAY); assert(result.value.array.data != NULL); assert(result.value.array.size == 0); json_free_value(&result); } { // One Element const char* string = "[\"Hello World\"]"; json_value result = { .type = TYPE_NULL }; assert(result.value.array.data == NULL); assert(json_parse_value(&string, &result)); assert(result.type = TYPE_ARRAY); assert(result.value.array.data != NULL); assert(result.value.array.size == 1); json_value* string_value = (json_value *)result.value.array.data; assert(string_value->type == TYPE_STRING); assert(strcmp("Hello World", string_value->value.string) == 0);; json_free_value(&result); } { // Mutliple Elements const char* string = "[0, 1, 2, 3]"; json_value result = { .type = TYPE_NULL }; assert(result.value.array.data == NULL); assert(json_parse_value(&string, &result)); assert(result.type = TYPE_ARRAY); assert(result.value.array.data != NULL); assert(result.value.array.size == 4); json_free_value(&result); } { // Failure const char* string = "[0, 2,,]"; json_value result = { .type = TYPE_NULL }; assert(result.value.array.data == NULL); assert(result.type == TYPE_NULL); assert(result.value.array.data == NULL); json_free_value(&result); } { // Failure // Shouldn"t need to free, valgrind shouldn"t show leak const char* string = "[0, 2, 0"; json_value result = { .type = TYPE_NULL }; assert(result.value.array.data == NULL); assert(result.type == TYPE_NULL); assert(result.value.array.data == NULL); } printf(" OK\n"); } void json_test_value_object(void) { printf("json_test_value_object: "); { // Empty Object const char* string = "{}"; json_value result = { .type = TYPE_NULL }; assert(result.value.object.data == NULL); assert(json_parse_value(&string, &result)); assert(result.type = TYPE_OBJECT); assert(result.value.array.data != NULL); assert(result.value.array.size == 0); json_free_value(&result); } { // One Pair const char* string = "{ \"a\" : 1 }"; json_value result = { .type = TYPE_NULL }; assert(result.value.object.data == NULL); assert(json_parse_value(&string, &result)); assert(result.type = TYPE_OBJECT); assert(result.value.array.data != NULL); assert(result.value.array.size == 2); json_value* members = (json_value *)result.value.object.data; assert(strcmp(json_value_to_string(members), "a") == 0); ++members; assert(json_value_to_double(members) == 1.0); json_free_value(&result); } { // Multiple Pairs const char* string = "{ \"a\": 1, \"b\" : 2, \"c\" : 3 }"; json_value result = { .type = TYPE_NULL }; assert(result.value.object.data == NULL); assert(json_parse_value(&string, &result)); assert(result.type = TYPE_OBJECT); assert(result.value.array.data != NULL); assert(result.value.array.size == 6); json_value* members = (json_value *)result.value.object.data; assert(strcmp(json_value_to_string(&members[4]), "c") == 0); assert(json_value_to_double(&members[5]) == 3.0); json_free_value(&result); } printf(" OK\n"); } void json_test_value_literal(void) { printf("json_test_values_literal: "); { const char* string = "true"; json_value result = { .type = TYPE_NULL }; assert(json_parse_value(&string, &result)); assert(result.type == TYPE_BOOL); assert(result.value.boolean); json_free_value(&result); } { const char* string = "false"; json_value result = { .type = TYPE_NULL }; assert(json_parse_value(&string, &result)); assert(result.type == TYPE_BOOL); assert(!result.value.boolean); json_free_value(&result); } { const char* string = "null"; json_value result = { .type = TYPE_NULL }; assert(json_parse_value(&string, &result)); assert(result.type == TYPE_NULL); json_free_value(&result); } printf(" OK\n"); } const char* test_string_valid = " \ { \"item1\" : [1, 2, 3, 4], \ \"item2\" : { \"a\" : 1, \"b\" : 2, \"c\" : 3 }, \ \"item3\" : \"An Item\" \ }"; const char* test_string_invalid = " \ { \"item1\" : [1, 2, 3, 4], \ \"item2\" : { \"a\" : 1, \"b\" : 2, \"c\" : 3 }, \ \"item3\" , \"An Item\" \ }"; void json_test_coarse(void) { printf("json_test_coarse: "); json_value root; assert(json_parse(test_string_valid, &root)); json_value* val = json_value_with_key(&root, "item1"); assert(root.type == TYPE_OBJECT); assert(root.value.object.size == 6); assert(val != NULL); assert(json_value_to_array(val) != NULL); assert(json_value_to_array(val)->size == 4); val = json_value_with_key(&root, "item3"); assert(val != NULL); assert(json_value_to_string(val) != NULL); assert(strcmp(json_value_to_string(val), "An Item") == 0); json_free_value(&root); // valgrind check for releasing intermediary data assert(!json_parse(test_string_invalid, &root)); printf(" OK\n"); } void json_test_all(void) { json_test_value_invalid(); json_test_value_string(); json_test_value_number(); json_test_value_array(); json_test_value_object(); json_test_value_literal(); json_test_coarse(); } #endif Risposta
Intestazione
In C, tutti i enum condividono lo stesso spazio dei nomi tra loro ( e con cose come nomi di variabili). È quindi una buona idea cercare di ridurre il rischio che si scontrino.
I tuoi nomi enum json_value_type hanno il prefisso TYPE_, che è piuttosto generico. Qualche altra libreria potrebbe provare a usare lo stesso nome. Suggerirei di cambiare il prefisso in, ad esempio, JSON_.
Inoltre, non sembra che tu stia utilizzando TYPE_KEY per qualsiasi cosa. Basta rimuoverlo.
Implementazione
Come osserva Roland Illig , gli argomenti per iscntrl() e isspace() nella funzione skip_whitespace() devono essere trasmessi a unsigned char a evitare lestensione del segno.
In alternativa, e seguendo più da vicino la specifica JSON , potresti riscrivere questa funzione semplicemente come:
static void skip_whitespace(const char** cursor) { while (**cursor == "\t" || **cursor == "\r" || **cursor == "\n" || **cursor == " ") ++(*cursor); } Molte delle tue static funzioni di supporto eseguono combinazioni non banali di cose e mancano di commenti che spieghino cosa fare. Una o due righe di commento prima di ogni funzione potrebbero aiutare molto la leggibilità.
In particolare, la tua funzione has_char() fa un sacco di cose diverse:
- Salta gli spazi.
- Controlla la presenza di un certo carattere nellinput.
- Se il carattere viene trovato, lo salta automaticamente.
Solo # 2 è ovviamente implicito nel nome della funzione; gli altri sono effetti collaterali inaspettati e dovrebbero almeno essere chiaramente documentati.
In realtà, mi sembra che potrebbe essere meglio rimuovere la chiamata a skip_whitespace() da has_char() e lascia che il chiamante salti esplicitamente gli spazi bianchi prima di chiamarlo se necessario. In molti casi il tuo codice lo fa già, rendendo il duplicato ridondante.
Inoltre, per rendere leffetto n. 3 meno sorprendente per il lettore, potrebbe essere una buona idea rinominare quella funzione in qualcosa di un po più attivo come, ad esempio, read_char().
Alla fine di json_parse_object(), hai:
return success; return 1; } Sicuramente è ridondante. Sbarazzati del return 1;.
Inoltre, sembra che tu stia usando la funzione generica json_parse_value() per analizzare le chiavi degli oggetti e non testare per assicurarti che “siano stringhe”. Ciò consente ad alcuni JSON non validi di passare attraverso il tuo parser. Suggerirei di aggiungere un controllo di tipo esplicito o di suddividere il codice di analisi della stringa in una funzione separata (come descritto di seguito) e di chiamarlo direttamente da json_parse_object().
Nella parte superiore di json_parse_array(), hai:
if (**cursor == "]") { ++(*cursor); return success; } while (success) { Potresti riscriverlo allo stesso modo di lo fai in json_parse_object():
while (success && !has_char("]")) { (Solo, sai, penso ancora che il nome read_char() sarebbe meglio.)
Inoltre, per qualche motivo, il tuo json_parse_array() sembra aspettarsi che il chiamante inizializzi il parent struct, mentre json_parse_object() lo fa automaticamente. AFAICT non cè motivo per lincongruenza, quindi potresti e probabilmente dovresti semplicemente far funzionare entrambe le funzioni allo stesso modo.
La funzione json_is_literal() non è contrassegnata come static, anche se “t appaiono nellintestazione. Come is_char(), io “d al quindi preferisci rinominarlo in qualcosa di più attivo, come json_read_literal() o semplicemente read_literal(), per rendere più chiaro che sposta automaticamente il cursore su un corrispondenza riuscita.
(Si noti inoltre che, come scritto, questa funzione non verifica che il valore letterale nellinput finisca effettivamente dove dovrebbe. Ad esempio, corrisponderebbe correttamente allinput nullnullnull con null.Non credo che sia un vero bug, poiché gli unici valori letterali validi in JSON sono true, false e null, nessuno dei quali è prefisso luno dellaltro, e poiché due letterali non possono apparire consecutivamente in un JSON valido senza qualche altro token intermedio. Ma è sicuramente almeno degno di nota in un commento.
Potresti anche contrassegnare esplicitamente alcune delle tue funzioni di supporto statiche come inline per dare al compilatore un suggerimento che dovrebbe provare a unirli nel codice chiamante. Suggerirei di farlo almeno per skip_whitespace(), has_char() e json_is_literal().
Poiché le tue funzioni di accesso json_value_to_X() non sono costituite da nientaltro che un assert() e una dereferenziazione del puntatore, dovresti anche considerare di spostare le loro implementazioni in json.h e contrassegnarle come static inline. Ciò consentirebbe al compilatore di incorporarli nel codice chiamante anche in altri .c file e possibilmente di ottimizzare assert() se la chiamata codice controlla già il tipo comunque.
Nella funzione json_parse() principale, potresti voler controllare esplicitamente che non ci siano altro che spazi vuoti nel input dopo che il valore di root è stato analizzato.
String parsing
Il codice di analisi della stringa in json_parse_value() non funziona, poiché non “t gestire gli escape con barra rovesciata. Ad esempio, non riesce con il seguente input JSON valido:
"I say: \"Hello, World!\"" Puoi aggiungerlo come caso di prova.
Tu dovresti anche verificare che il tuo codice gestisca correttamente altre sequenze di escape con barra rovesciata come \b, \f, \n, \r, \t, \/ e soprattutto \\ e \unnnn. Ecco alcuni altri casi di test per questi:
"\"\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" Poiché le stringhe JSON possono contenere caratteri Unicode arbitrari, dovrai decidere come gestirli. Probabilmente la scelta più semplice sarebbe dichiarare linput e loutput in UTF-8 (o forse WTF-8 ) e convertire \unnnn esegue lescape in sequenze di byte UTF-8 (e, facoltativamente, viceversa). Tieni presente che, poiché stai utilizzando stringhe con terminazione null, potresti preferire decodificare \u0000 in la codifica troppo lunga "\xC0\x80" invece di un byte nullo.
Per mantenere leggibile la funzione json_parse_value() principale, Consiglio vivamente di dividere il codice di analisi delle stringhe in una funzione di supporto separata. Soprattutto perché farlo gestire correttamente le sequenze di backslash lo complicherà notevolmente.
Una delle complicazioni è che non saprai effettivamente per quanto tempo il sarà fino a quando non lavrai analizzata. Un modo per gestirlo sarebbe aumentare dinamicamente la stringa di output allocata con realloc(), ad esempio in questo modo:
// 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; } Una soluzione alternativa sarebbe quella di eseguire due passaggi di analisi sullinput: uno solo per determinare la lunghezza della stringa di output e un altro per decodificarla effettivamente. Questo sarebbe più semplice qualcosa del genere:
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; } dove la funzione di supporto:
static int json_parse_helper(const char** cursor, size_t* length, char* new_string) { // ... } analizza una stringa JSON di al massimo *length byte in new_string e scrive la lunghezza effettiva della stringa analizzata in *length, oppure se new_string == NULL, determina solo la lunghezza della stringa senza effettivamente memorizzare loutput decodificato da nessuna parte.
Analisi del numero
Il tuo json_parse_value() limplementazione considera i numeri come le maiuscole / minuscole e semplicemente alimenta tutto ciò che “non è con ", [, {, n, t o f nella funzione di libreria standard C strtod().
Poiché strtod() accetta un superset di numeri letterali JSON validi, dovrebbe funzionare, ma a volte può rendere il tuo codice ac cept JSON non valido come valido. Ad esempio, il tuo codice accetterà +nan, -nan, +inf e -inf come numeri validi e accetterà anche notazioni esadecimali come 0xABC123. Inoltre, come la strtod() documentazione collegata sopra le note:
In una lingua diversa dalla “C” standard o “POSIX”, questa funzione può riconoscere una sintassi aggiuntiva dipendente dalla locale.
Se vuoi essere più rigoroso, potresti voler convalidare esplicitamente tutto ciò che assomiglia a un numero rispetto a grammatica JSON prima di passarla a strtod().
Tieni inoltre presente che strtod() può impostare errno ad es se il numero di input è esterno allintervallo di double. Probabilmente dovresti controllarlo.
Test
Non ho esaminato i tuoi test in dettaglio, ma è fantastico vedere che li hai (anche se, come notato sopra, la loro copertura potrebbe essere migliorata).
Personalmente, però, preferirei spostare i test fuori dallimplementazione in un file sorgente separato. Questo ha sia vantaggi che svantaggi:
- Il principale svantaggio è che non puoi più testare direttamente le funzioni di aiuto statiche. Tuttavia, dato che la tua API pubblica sembra pulita e completa e non soffre di problemi di “stato nascosto” che complicherebbero i test, dovresti essere in grado di ottenere una buona copertura del test anche solo tramite lAPI.
- Il vantaggio principale (oltre a una netta separazione tra implementazione e codice di test) è che i tuoi test testeranno automaticamente lAPI pubblica. In particolare, eventuali problemi con lintestazione
json.hverranno visualizzati in i tuoi test. Inoltre, eseguire i test tramite lAPI ti aiuta a garantire che la tua API sia davvero sufficientemente completa e flessibile per un uso generale.
Se vuoi ancora testare direttamente le tue funzioni statiche, potresti sempre aggiungere un flag del preprocessore che facoltativamente li esponga per il test, tramite semplici wrapper o semplicemente rimuovendo la parola chiave static dalle loro definizioni.
Ps. I ha notato che il tuo test json_test_value_number() non riesce per me (GCC 5.4.0, i386 arch), presumibilmente perché il numero 23,4 non è esattamente rappresentabile in virgola mobile. Cambiarlo a 23.5 rende il test superato.
Commenti
Risposta
Questa non è in alcun modo una recensione completa, ma condividerò alcune cose che hanno attirato la mia attenzione durante la lettura del tuo codice.
Commenti
Mentre i commenti sono sicuramente piacevoli, alcuni dei tuoi commenti in linea aggiungono solo rumore al codice.
// Eat whitespace int success = 0; skip_whitespace(cursor); Prima di tutto, il commento è una riga troppo presto. Secondo, si può leggere che lo spazio bianco viene consumato guardando la funzione – il nome la descrive perfettamente, th Non cè bisogno di un commento aggiuntivo.
case "\0": // If parse_value is called with the cursor at the end of the string // that"s a failure success = 0; break; Di nuovo, questo commento si limita a ripetere ciò che dice il codice stesso.
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 }; Ora, questi commenti non sono realmente inutili poiché documentano ciò che rappresenta ciascun valore. Ma perché solo per TYPE_OBJECT e TYPE_ARRAY, perché non per tutti i valori? Personalmente, ho appena inserito un link a json.org appena prima di enum. I tuoi tipi sono analoghi a quelli lì, devi solo documentare cosa dovrebbe essere TYPE_KEY. Il che mi porta al punto successivo …
TYPE_KEY
Dando uno sguardo a json.org , puoi vedere un oggetto costituito da un elenco di membri , che a loro volta sono composti da una stringa e un valore . Il che significa che non hai davvero bisogno di TYPE_KEY! Basta aggiungere una nuova struttura per membri composta da un valore TYPE_STRING e un altro valore json di qualsiasi tipo e sei a posto. In questo momento, potresti avere ad esempio un numero come chiave per un valore, che non è consentito. Renderebbe anche più gradevole la logica relativa agli oggetti, come questo per il ciclo:
for (size_t i = 0; i < size; i += 2) Ironia della sorte, il passaggio di questo ciclo for potrebbe effettivamente utilizzare un commento (perché += 2?) Ma ne manca uno.
Varie
case "\0": // If parse_value is called with the cursor at the end of the string // that"s a failure success = 0; break; Perché non solo return 0;?
while (iscntrl(**cursor) || isspace(**cursor)) ++(*cursor); e
if (success) ++(*cursor); e
if (has_char(cursor, "}")) break; else if (has_char(cursor, ",")) continue; e pochi altri di questi. Non lo sono particolarmente appassionato di mettere condizione e affermazione sulla stessa linea, soprattutto perché non lo fai in modo coerente.Mi piace fare questo per amore del flusso di controllo, come if (!something) return;, ma è ancora “meh”. Meglio farlo bene e mettere listruzione su una nuova riga.
Inoltre, trovo che il tuo codice potrebbe utilizzare alcune righe più vuote per separare le “regioni” o come vuoi chiamarle . Ad esempio:
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; Cè una riga vuota che separa le cose di configurazione e analisi da quelle di verifica e restituzione, ma puoi farlo meglio.
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; Trovo che sia molto più pulito. Hai un blocco per impostare i valori, un blocco per analizzarli, un blocco per metterli nel vettore, un blocco per saltare gli spazi e un blocco per finalizzare lazione corrente. Lultima riga vuota tra skip_whitespace(cursor); e if ... è discutibile , ma preferisco così.
A parte questo, ho trovato il tuo codice facilmente leggibile e comprensibile. Verifichi correttamente eventuali errori e usi nomi ragionevoli. Per quanto riguarda lidiomaticità, a parte da quanto ho detto, non cè niente che io contrassegni come insolito o non idomatico.
Risposta
Le funzioni di ctype.h non devono essere chiamate con argomenti di tipo char, poiché ciò può invocare un comportamento indefinito. Consulta la documentazione di NetBSD per una buona spiegazione.
\u00XX, dal momento che alcuni codificatori potrebbero scegliere di usarli anche per i caratteri ASCII. (Inoltre, ho aggiunto un suggerimento per contrassegnare alcune delle tue funzioni comeinlinesopra, poiché ho dimenticato di farlo prima .)