Hier is een eenvoudige JSON-parser met recursieve afdaling, niet veel extra functionaliteit, hoewel het wel de uitbreidbare vectorklasse gebruikt die hier wordt besproken ( Eenvoudige uitbreidbare vector in C ). Ik heb geen optimalisaties geïmplementeerd, noch een toegangsinterface op een hoger niveau, het is allemaal vrij eenvoudig. Er is ook geen JSON-export, alleen importeren.
De hele bron is beschikbaar op github ( https://github.com/HarryDC/JsonParser ). CMake en testcode inbegrepen, ik zal de parserbestanden hier gewoon plaatsen.
Mijn voornaamste interesse zou zijn als er meer idiomatische manieren zijn om dingen in C te schrijven. Maar natuurlijk wordt elke andere input ook altijd gewaardeerd.
Header
#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 Implementatie
#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 Antwoord
Header
In C delen alle enum namen dezelfde naamruimte met elkaar ( en met zaken als variabelenamen). Het is daarom een goed idee om te proberen het risico te verkleinen dat ze “botsen.
Uw enum json_value_type namen hebben het voorvoegsel TYPE_, wat vrij algemeen is. Een andere bibliotheek probeert misschien dezelfde naam te gebruiken. Ik zou willen voorstellen dat voorvoegsel te wijzigen in bijvoorbeeld JSON_.
Ook lijkt het erop dat je geen TYPE_KEY voor wat dan ook. Verwijder het gewoon.
Implementatie
Zoals Roland Illig opmerkt , de argumenten voor iscntrl() en isspace() in uw skip_whitespace() functie moeten worden gecast naar unsigned char naar vermijd tekenuitbreiding.
Als alternatief, en nauwkeuriger de JSON-specificatie te volgen, kunt u deze functie eenvoudig herschrijven als:
static void skip_whitespace(const char** cursor) { while (**cursor == "\t" || **cursor == "\r" || **cursor == "\n" || **cursor == " ") ++(*cursor); } Veel van uw static helperfuncties doen niet-triviale combinaties van dingen, en hebben geen commentaar waarin wordt uitgelegd wat ze Doen. Een of twee commentaarregels voor elke functie kunnen de leesbaarheid aanzienlijk verbeteren.
In het bijzonder doet uw has_char() -functie een heleboel verschillende dingen:
- Het slaat spaties over.
- Het controleert op de aanwezigheid van een bepaald teken in de invoer.
- Als het teken wordt gevonden, wordt het automatisch overgeslagen.
Alleen # 2 wordt duidelijk geïmpliceerd door de functienaam; de andere zijn onverwachte bijwerkingen en moeten in ieder geval duidelijk worden gedocumenteerd.
Eigenlijk lijkt het mij beter om de aanroep naar skip_whitespace() te verwijderen van has_char(), en laat de beller expliciet witruimte overslaan voordat hij hem aanroept, indien nodig. In veel gevallen doet uw code dat al, waardoor het duplicaat overbodig wordt.
Om effect # 3 minder verrassend te maken voor de lezer, is het misschien een goed idee om die functie te hernoemen naar iets meer actief zoals bijvoorbeeld read_char().
Aan het einde van json_parse_object() heb je:
return success; return 1; } Dat is zeker overbodig. Verwijder gewoon de return 1;.
Ook, het lijkt erop dat je de generieke json_parse_value() -functie gebruikt om objectsleutels te parseren, en niet test om er zeker van te zijn dat ze strings zijn. Hierdoor kan een ongeldige JSON door uw parser komen. Ik zou voorstellen om ofwel een expliciete typecontrole toe te voegen of je string-parseercode te splitsen in een aparte functie (zoals hieronder beschreven) en deze rechtstreeks aan te roepen vanuit json_parse_object().
Bovenaan json_parse_array() heb je:
if (**cursor == "]") { ++(*cursor); return success; } while (success) { Je zou dat op dezelfde manier kunnen herschrijven als je doet in json_parse_object():
while (success && !has_char("]")) { (Alleen, weet je, ik denk nog steeds dat de naam read_char() zou beter zijn.)
Om de een of andere reden lijkt uw json_parse_array() te verwachten dat de beller de parent struct, terwijl json_parse_object() het automatisch doet. AFAICT er is geen reden voor de inconsistentie, dus je zou en waarschijnlijk moeten beide functies laten werken op dezelfde manier.
Uw json_is_literal() -functie is niet gemarkeerd als static, ook al is het niet “t verschijnen in de koptekst. Net als is_char(), ik “d al geef het dus liever een andere naam in iets actievers, zoals json_read_literal() of gewoon read_literal(), om het duidelijker te maken dat de cursor automatisch naar een succesvolle match.
(Merk ook op dat, zoals geschreven, deze functie niet controleert of de letterlijke in de invoer daadwerkelijk eindigt waar het hoort. Het zou bijvoorbeeld met succes de invoer nullnullnull matchen met null.Ik denk niet dat dit een echte bug is, aangezien de enige geldige letterlijke waarden in JSON true, false en null, die geen voorvoegsels van elkaar zijn, en aangezien twee letterlijke letters niet opeenvolgend in geldige JSON kunnen voorkomen zonder een ander token ertussen. Maar het is zeker de moeite waard om in een opmerking op te merken.)
Misschien wil je ook expliciet een aantal van je statische helperfuncties markeren als inline om de compiler een hint te geven dat hij zou moeten proberen ze samen te voegen in de aanroepcode. Ik “zou willen voorstellen dat in ieder geval te doen voor skip_whitespace(), has_char() en json_is_literal().
Aangezien uw json_value_to_X() accessorfuncties allemaal bestaan uit niets anders dan een assert() en een pointer dereferentie, zou je ook moeten overwegen om hun implementaties naar json.h te verplaatsen en ze te markeren als static inline. Dit zou de compiler in staat stellen om ze in de aanroepende code in te lijnen, zelfs in andere .c bestanden, en mogelijk om de assert() weg te optimaliseren als de aanroep code controleert het type toch al.
In je hoofdfunctie json_parse() wil je misschien expliciet controleren of er niets dan witruimte over is in de invoer nadat de root-waarde is geparseerd.
String-parsing
Uw string-parseercode in json_parse_value() is verbroken, omdat het niet “t handvat backslash ontsnapt. Het mislukt bijvoorbeeld bij de volgende geldige JSON-invoer:
"I say: \"Hello, World!\"" Misschien wilt u dat toevoegen als een testcase.
U moet ook testen of uw code andere backslash-escape-reeksen correct verwerkt, zoals \b, \f, \n, \r, \t, \/ en vooral \\ en \unnnn. Hier zijn nog een paar testcases voor:
"\"\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" Aangezien JSON-strings willekeurige Unicode-tekens kunnen bevatten, moet u beslissen hoe u ermee omgaat. Waarschijnlijk is de eenvoudigste keuze om uw invoer en uitvoer in UTF-8 (of misschien WTF-8 ) te declareren en ontsnapt in UTF-8-byte-reeksen (en optioneel vice versa). Houd er rekening mee dat, aangezien u “strings gebruikt die op null eindigen, u er misschien de voorkeur aan geeft \u0000 te decoderen naar de te lange codering "\xC0\x80" in plaats van een nulbyte.
Om de hoofdfunctie json_parse_value() leesbaar te houden, Ik zou sterk aanbevelen om de string-parseercode op te splitsen in een aparte helperfunctie. Vooral omdat het behoorlijk ingewikkeld maken als je backslash-escapes correct afhandelt.
Een van de complicaties is dat je eigenlijk niet weet hoe lang de string zal zijn totdat je het hebt geparseerd. Een manier om daarmee om te gaan is door de toegewezen output string dynamisch te laten groeien met realloc(), bijvoorbeeld als volgt:
// 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; } Een alternatieve oplossing zou zijn om twee parsing-passages over de invoer uit te voeren: één om de lengte van de uitvoerstring te bepalen, en een andere om deze daadwerkelijk te decoderen. Dit zou het gemakkelijkst zijn. zoiets als dit:
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; } waarbij de helperfunctie:
static int json_parse_helper(const char** cursor, size_t* length, char* new_string) { // ... } parseert een JSON-string van maximaal *length bytes in new_string en schrijft de werkelijke lengte van de geparseerde string naar *length, of, if new_string == NULL, bepaalt gewoon de lengte van de string zonder daadwerkelijk de gedecodeerde uitvoer ergens op te slaan.
Getal parsing
Je huidige json_parse_value() implementatie behandelt getallen als het standaardgeval, en voedt eenvoudig alles wat niet “niet met ", [, {, n, t of f in de C-standaard bibliotheekfunctie strtod().
Aangezien strtod() een superset van geldige JSON-letterlijke getallen, dit zou moeten werken, maar kan uw code soms ac cept ongeldige JSON als geldig. Uw code accepteert bijvoorbeeld +nan, -nan, +inf en -inf als geldige getallen, en accepteert ook hexadecimale notatie zoals 0xABC123. Eveneens, aangezien de strtod() documentatie die hierboven is gelinkt opmerkt:
In een andere landinstelling dan de standaard “C” of “POSIX” landinstellingen, deze functie herkent mogelijk aanvullende landinstelling-afhankelijke syntaxis.
Als je strenger wilt zijn, wil je misschien expliciet alles valideren dat op een getal lijkt tegen de JSON-grammatica voordat u deze doorgeeft aan strtod().
Merk ook op dat strtod() errno bijv als het invoernummer buiten het bereik van een double valt. Je zou dit waarschijnlijk moeten controleren.
Testen
Ik heb je tests niet in detail bekeken, maar het is geweldig om te zien dat je ze hebt (zelfs als, zoals vermeld hierboven zou hun dekking kunnen worden verbeterd).
Persoonlijk echter, zou ik liever de tests uit de implementatie naar een apart bronbestand verplaatsen. Dit heeft zowel voor- als nadelen:
- Het grootste nadeel is dat je statische helperfuncties niet meer direct kunt testen. Aangezien uw openbare API er echter schoon en uitgebreid uitziet en geen “verborgen status” -problemen heeft die het testen bemoeilijken, zou u in staat moeten zijn om een goede testdekking te krijgen, zelfs alleen via de API.
- Het belangrijkste voordeel (naast een duidelijke scheiding tussen implementatie en testcode) is dat uw tests automatisch de openbare API testen. Met name eventuele problemen met de
json.h-header verschijnen in uw tests. Door uw tests via de API te doen, kunt u ervoor zorgen dat uw API echt voldoende compleet en flexibel is voor algemeen gebruik.
Als u uw statische functies echt nog steeds rechtstreeks wilt testen, je zou altijd een preprocessor-vlag kunnen toevoegen die ze optioneel blootstelt voor testen, ofwel via eenvoudige wrappers of door gewoon het static sleutelwoord uit hun definities te verwijderen.
Ps. I heeft gemerkt dat uw json_test_value_number() -test voor mij mislukt (GCC 5.4.0, i386 arch), vermoedelijk omdat het getal 23,4 niet exact kan worden weergegeven in drijvende komma. Door het te veranderen in 23.5 is de test geslaagd.
Reacties
Answer
Dit is op geen enkele manier een volledige recensie, maar ik zal enkele dingen delen die me opvielen tijdens het lezen van je code.
Opmerkingen
Terwijl opmerkingen zijn zeker leuk, sommige van uw inline opmerkingen voegen alleen ruis toe aan de code.
// Eat whitespace int success = 0; skip_whitespace(cursor); Allereerst is de opmerking een regel te vroeg. Ten tweede, men kan lezen dat de witruimte wordt verbruikt door naar de functie te kijken – de naam beschrijft het perfect, th Er is geen aanvullende opmerking nodig.
case "\0": // If parse_value is called with the cursor at the end of the string // that"s a failure success = 0; break; Nogmaals, deze opmerking herhaalt alleen wat de code zelf zegt.
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 }; Nu, deze commentaren zijn niet echt nutteloos aangezien ze documenteren wat elke waarde vertegenwoordigt. Maar waarom alleen voor TYPE_OBJECT en TYPE_ARRAY – waarom niet voor alle waarden? Persoonlijk “had ik net daarvoor een link naar json.org geplaatst enum. Uw typen zijn analoog aan degenen daar, je hoeft alleen te documenteren wat TYPE_KEY zou moeten zijn. Dat brengt me bij het volgende punt …
TYPE_KEY
Als je json.org bekijkt, kun je zien dat een object bestaat uit een lijst met leden , die op hun beurt zijn gemaakt van een string en een waarde . Wat betekent dat je TYPE_KEY! Voeg gewoon een nieuwe structuur toe voor leden bestaande uit een TYPE_STRING -waarde en een andere json-waarde van elk type en je bent klaar om te gaan. Op dit moment zou je kunnen heb bijv. een nummer als sleutel voor een waarde, wat niet is toegestaan. Zou een deel van de objectgerelateerde logica ook leuker maken, zoals dit voor lus:
for (size_t i = 0; i < size; i += 2) Ironisch genoeg zou de stap van deze for-lus eigenlijk een opmerking kunnen gebruiken (waarom += 2?), Maar er ontbreekt er een.
Diversen
case "\0": // If parse_value is called with the cursor at the end of the string // that"s a failure success = 0; break; Waarom niet gewoon return 0;?
while (iscntrl(**cursor) || isspace(**cursor)) ++(*cursor); en
if (success) ++(*cursor); en
if (has_char(cursor, "}")) break; else if (has_char(cursor, ",")) continue; en een paar andere daarvan. Ik “niet vooral gesteld om conditie en statement op één lijn te brengen, vooral omdat je dit niet consequent doet.Ik vind het een beetje oké om dit te doen omwille van de controle, zoals if (!something) return;, maar het is nog steeds “bleh”. Het is beter om het goed te doen en de verklaring op een nieuwe regel te plaatsen.
Ook vind ik dat uw code wat meer lege regels kan gebruiken om “regios” te scheiden of hoe u ze ook wilt noemen . Bijvoorbeeld:
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; Er is één lege regel die de setup-en-parse-dingen scheidt van de check-and-return-dingen, maar je kunt doen beter.
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; Ik vind dit veel schoner. Je hebt een blok voor het instellen van de waarden, een blok om ze te ontleden, een blok om ze te plaatsen in de vector, een blok om witruimte over te slaan en een blok om de huidige actie af te ronden. De laatste lege regel tussen skip_whitespace(cursor); en if ... is discutabel , maar ik geef er de voorkeur aan op deze manier.
Verder vond ik dat uw code gemakkelijk leesbaar en begrijpelijk is. U controleert goed op eventuele fouten en gebruikt verstandige naamgeving. Wat betreft de idiomaticiteit, afgezien van wat ik heb genoemd, is er niets dat ik zou markeren als ongebruikelijk of niet-idomatisch.
Answer
De functies van ctype.h mogen niet worden aangeroepen met argumenten van het type char, aangezien dat ongedefinieerd gedrag kan oproepen. Zie de NetBSD-documentatie voor een goede uitleg.
\u00XX, aangezien sommige encoders ervoor kiezen om ze zelfs voor ASCII-tekens te gebruiken. (Ik heb ook een suggestie toegevoegd om sommige van je functies hierboven alsinlinete markeren, aangezien ik dat eerder vergat .)