Aquí hay un analizador JSON de descenso recursivo simple, sin mucha funcionalidad adicional, aunque usa la clase de vector expandible revisada aquí ( Vector expandible simple en C ). No implementé ninguna optimización, ni una interfaz de acceso de nivel superior, todo es bastante básico. Tampoco hay una exportación JSON, solo una importación.
Toda la fuente está disponible en github ( https://github.com/HarryDC/JsonParser ). CMake y código de prueba incluido, solo publicaré los archivos del analizador aquí.
Mi principal interés sería si hay formas más idiomáticas de escribir cosas en C. Pero, por supuesto, cualquier otra entrada también se agradece.
Encabezado
#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
Implementación
#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
Respuesta
Encabezado
En C, todos los enum
comparten el mismo espacio de nombres entre sí ( y con cosas como nombres de variables). Por lo tanto, es una buena idea tratar de reducir el riesgo de que «choquen».
Sus nombres enum json_value_type
tienen el prefijo TYPE_
, que es bastante genérico. Es posible que alguna otra biblioteca intente utilizar el mismo nombre. Sugeriría cambiar ese prefijo a, digamos, JSON_
.
Además, no parece que estés usando TYPE_KEY
para cualquier cosa. Simplemente elimínelo.
Implementación
Como señala Roland Illig , los argumentos para iscntrl()
y isspace()
en tu skip_whitespace()
la función se debe convertir a unsigned char
para evitar la extensión de signo.
Alternativamente, y siguiendo más de cerca la especificación JSON , puede reescribir esta función simplemente como:
static void skip_whitespace(const char** cursor) { while (**cursor == "\t" || **cursor == "\r" || **cursor == "\n" || **cursor == " ") ++(*cursor); }
Muchas de sus static
funciones auxiliares hacen combinaciones no triviales de cosas y carecen de comentarios que expliquen lo que hacer. Una o dos líneas de comentarios antes de cada función podrían ayudar mucho a la legibilidad.
En particular, su función has_char()
hace un montón de cosas diferentes:
- Omite los espacios en blanco.
- Comprueba la presencia de un determinado carácter en la entrada.
- Si se encuentra el carácter, lo omite automáticamente.
Obviamente, el nombre de la función sólo implica el número 2; los otros son efectos secundarios inesperados, y al menos deberían estar claramente documentados.
De hecho, me parece que sería mejor eliminar la llamada a skip_whitespace()
from has_char()
, y deje que la persona que llama omita explícitamente los espacios en blanco antes de llamarlo si es necesario. En muchos casos, su código ya hace eso, lo que hace que el salto duplicado sea redundante.
Además, para que el efecto n. ° 3 sea menos sorprendente para el lector, podría ser una buena idea cambiar el nombre de esa función a algo un poco más activo como, digamos, read_char()
.
Al final de json_parse_object()
, tienes:
return success; return 1; }
Seguramente eso es redundante. Simplemente elimine el return 1;
.
Además, parece que estás usando la función json_parse_value()
genérica para analizar las claves de los objetos, y no pruebas para asegurarte de que son cadenas. Esto permite que un JSON inválido pase a través de su analizador. Sugeriría agregar una verificación de tipo explícita o dividir el código de análisis de cadenas en una función separada (como se describe a continuación) y llamarlo directamente desde json_parse_object()
.
En la parte superior de json_parse_array()
, tienes:
if (**cursor == "]") { ++(*cursor); return success; } while (success) {
Podrías reescribirlo de la misma manera que lo haces en json_parse_object()
:
while (success && !has_char("]")) {
(Solo, ya sabes, todavía creo que el nombre read_char()
sería mejor.)
Además, por alguna razón, su json_parse_array()
parece esperar que la persona que llama inicialice el parent
struct, mientras que json_parse_object()
lo hace automáticamente. AFAICT no hay ninguna razón para la inconsistencia, por lo que podría y probablemente debería hacer que ambas funciones funcionen correctamente. de la misma manera.
Su función json_is_literal()
no está marcada como static
, aunque no aparecen en el encabezado. Como is_char()
, «marqué así que prefiero cambiarle el nombre a algo más activo, como json_read_literal()
o simplemente read_literal()
, para que quede más claro que avanza automáticamente el cursor en un coincidencia exitosa.
(También tenga en cuenta que, como está escrito, esta función no verifica que el literal en la entrada realmente termine donde se supone que debe hacerlo. Por ejemplo, coincidiría correctamente con la entrada nullnullnull
con null
.No creo que sea un error real, ya que los únicos literales válidos en JSON son true
, false
y null
, ninguno de los cuales son prefijos entre sí, y dado que dos literales no pueden aparecer consecutivamente en JSON válido sin algún otro token en el medio. Pero definitivamente vale la pena mencionarlo en un comentario.)
También puede marcar explícitamente algunas de sus funciones auxiliares estáticas como inline
para darle al compilador una pista de que debería intentar fusionarlos en el código de llamada. Sugeriría hacerlo al menos para skip_whitespace()
, has_char()
y json_is_literal()
.
Dado que sus json_value_to_X()
funciones de acceso no consisten en nada más que un assert()
y una desreferencia de puntero, también debería considerar mover sus implementaciones a json.h
y marcarlas como static inline
. Esto permitiría al compilador insertarlos en el código de llamada incluso en otros archivos .c
, y posiblemente optimizar assert()
si la llamada el código ya comprueba el tipo de todos modos.
En su función principal json_parse()
, es posible que desee comprobar explícitamente que no quedan más que espacios en blanco en el después de analizar el valor raíz.
Análisis de cadenas
El código de análisis de cadenas en json_parse_value()
está roto, ya que no manejar los escapes de barra invertida. Por ejemplo, falla en la siguiente entrada JSON válida:
"I say: \"Hello, World!\""
Es posible que desee agregar eso como un caso de prueba.
Usted también debe probar que su código maneja correctamente otras secuencias de escape de barra invertida como \b
, \f
, \n
, \r
, \t
, \/
y especialmente \\
y \unnnn
. Aquí hay algunos casos de prueba más para esos:
"\"\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"
Dado que las cadenas JSON pueden contener caracteres Unicode arbitrarios, deberá decidir cómo manejarlos. Probablemente, la opción más simple sería declarar su entrada y salida en UTF-8 (o quizás WTF-8 ) y convertir \unnnn
escapa a secuencias de bytes UTF-8 (y, opcionalmente, viceversa). Tenga en cuenta que, dado que está utilizando cadenas terminadas en nulo, es posible que prefiera decodificar \u0000
en la codificación excesiva "\xC0\x80"
en lugar de un byte nulo.
Para mantener la función principal json_parse_value()
legible, Recomiendo encarecidamente dividir el código de análisis de cadenas en una función auxiliar separada. Especialmente porque hacer que maneje los escapes de barra invertida correctamente lo complicará considerablemente.
Una de las complicaciones es que no sabrá realmente cuánto tiempo durará el string será hasta que lo haya analizado. Una forma de lidiar con eso sería hacer crecer dinámicamente la cadena de salida asignada con realloc()
, por ejemplo, así:
// 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 solución alternativa sería ejecutar dos pasadas de análisis sobre la entrada: una solo para determinar la longitud de la cadena de salida y otra para decodificarla realmente. Esto sería más fácil de hacer algo como esto:
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; }
donde la función auxiliar:
static int json_parse_helper(const char** cursor, size_t* length, char* new_string) { // ... }
analiza una cadena JSON de como máximo *length
bytes en new_string
y escribe la longitud real de la cadena analizada en *length
, o if new_string == NULL
, simplemente determina la longitud de la cadena sin almacenar la salida decodificada en ningún lugar.
Análisis numérico
Su json_parse_value()
La implementación trata los números como el caso predeterminado y simplemente alimenta cualquier cosa que no esté con "
, [
, {
, n
, t
o f
en la función de biblioteca estándar de C strtod()
.
Dado que strtod()
acepta un superconjunto de literales de números JSON válidos, esto debería funcionar, pero puede hacer que su código a veces sea ac excepto JSON no válido como válido. Por ejemplo, su código aceptará +nan
, -nan
, +inf
y -inf
como números válidos y también aceptará notación hexadecimal como 0xABC123
. Además, como señala la documentación strtod()
vinculada anteriormente:
En una configuración regional distinta de la «C» estándar o locales «POSIX», esta función puede reconocer sintaxis adicional dependiente de la localización.
Si desea ser más estricto, es posible que desee validar explícitamente cualquier cosa que parezca un número con el gramática JSON antes de pasarla a strtod()
.
También tenga en cuenta que strtod()
puede establecer errno
p. ej. si el número de entrada está fuera del rango de double
. Probablemente debería comprobar esto.
Probando
No he examinado sus pruebas en detalle, pero es genial ver que las tiene (incluso si, como se indicó anterior, su cobertura podría mejorarse).
Personalmente, sin embargo, preferiría mover las pruebas fuera de la implementación a un archivo fuente separado. Esto tiene ventajas y desventajas:
- La principal desventaja es que ya no puede probar directamente las funciones auxiliares estáticas. Sin embargo, dado que su API pública parece limpia y completa, y no sufre de ningún problema de «estado oculto» que complicaría las pruebas, debería poder lograr una buena cobertura de prueba incluso a través de la API.
- La principal ventaja (además de una clara separación entre la implementación y el código de prueba) es que tus pruebas probarán automáticamente la API pública. En particular, cualquier problema con el encabezado
json.h
se mostrará en sus pruebas. Además, hacer sus pruebas a través de la API le ayuda a asegurarse de que su API sea lo suficientemente completa y flexible para el uso general.
Si todavía desea probar directamente sus funciones estáticas, siempre puede agregar un indicador de preprocesador que, opcionalmente, los exponga para pruebas, ya sea mediante envoltorios simples o simplemente eliminando la palabra clave static
de sus definiciones.
Ps. I noté que tu json_test_value_number()
prueba falla para mí (GCC 5.4.0, arco i386), presumiblemente porque el número 23,4 no se puede representar exactamente en coma flotante. Cambiarlo a 23.5 hace que la prueba pase.
Comentarios
Responder
Esto de ninguna manera es una revisión completa, pero compartiré algunas cosas que me llamaron la atención mientras leía su código.
Comentarios
Mientras los comentarios seguramente son agradables, algunos de sus comentarios en línea solo agregan ruido al código.
// Eat whitespace int success = 0; skip_whitespace(cursor);
En primer lugar, el comentario es una línea demasiado pronto. En segundo lugar, se puede leer que el espacio en blanco se consume al mirar la función; el nombre lo describe perfectamente, th No es necesario un comentario adicional.
case "\0": // If parse_value is called with the cursor at the end of the string // that"s a failure success = 0; break;
De nuevo, este comentario simplemente repite lo que dice el código en sí.
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 };
Ahora bien, estos comentarios no son realmente inútiles ya que documentan lo que representa cada valor. Pero, ¿por qué solo para TYPE_OBJECT
y TYPE_ARRAY
? ¿Por qué no para todos los valores? Personalmente, simplemente pondría un vínculo a json.org justo antes de enum
. Tus tipos son análogos a los que están allí, solo necesita documentar lo que se supone que es TYPE_KEY
. Lo que me lleva al siguiente punto …
TYPE_KEY
Si echa un vistazo a json.org , puede ver un objeto que consta de una lista de miembros , que a su vez están formados por una cadena y un valor . Lo que significa que realmente no necesitas TYPE_KEY
! Simplemente agregue una nueva estructura para miembros que consta de un valor TYPE_STRING
y otro valor json de cualquier tipo y estará listo. Ahora mismo, podría tener, por ejemplo, un número como clave para un valor, que no está permitido. También haría algo de la lógica relacionada con el objeto más agradable, como este para el ciclo:
for (size_t i = 0; i < size; i += 2)
Irónicamente, el paso de este ciclo for en realidad podría usar un comentario (¿por qué += 2
?) Pero carece de uno.
Varios
case "\0": // If parse_value is called with the cursor at the end of the string // that"s a failure success = 0; break;
¿Por qué no simplemente return 0;
?
while (iscntrl(**cursor) || isspace(**cursor)) ++(*cursor);
y
if (success) ++(*cursor);
y
if (has_char(cursor, "}")) break; else if (has_char(cursor, ",")) continue;
y algunos otros de esos. No particularmente aficionado a poner la condición y la declaración en la misma línea, especialmente porque no está haciendo esto constantemente.Estoy un poco de acuerdo con hacer esto por el bien del flujo de control, como if (!something) return;
, pero sigue siendo «meh». Mejor hazlo bien y coloca la declaración en una nueva línea.
Además, encuentro que tu código podría usar algunas líneas más vacías para separar «regiones» o como quieras llamarlas . Por ejemplo:
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;
Hay una línea vacía que separa los elementos de configuración y análisis de los elementos de verificación y devolución, pero puede hacerlo mejor.
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;
Creo que esto es mucho más limpio. Tienes un bloque para configurar los valores, un bloque para analizarlos, un bloque para ponerlos en el vector, un bloque para omitir espacios en blanco y un bloque para finalizar la acción actual. La última línea vacía entre skip_whitespace(cursor);
y if ...
es discutible , pero yo lo prefiero de esta manera.
Aparte de eso, encontré que su código es fácilmente legible y comprensible. Verifica adecuadamente cualquier error y usa nombres razonables. En cuanto a la idiomaticidad, aparte por lo que he mencionado, no hay nada que marcaría como inusual o poco idomático.
Respuesta
Las funciones de ctype.h
no deben llamarse con argumentos de tipo char
, ya que eso puede invocar un comportamiento indefinido. Consulte la documentación de NetBSD para obtener una buena explicación.
\u00XX
, ya que algunos codificadores pueden optar por usarlos incluso para caracteres ASCII. (Además, agregué una sugerencia para marcar algunas de sus funciones comoinline
arriba, ya que olvidé hacerlo antes .)