patterncModerate
parcel: a JSON parsing library in C
Viewed 0 times
jsonparcelparsinglibrary
Problem
I've created a JSON parsing library in C, and would like some feedback on it (feel free to submit a pull request on GitHub). Any and all suggestions are acceptable, but I would prefer if reviews were focused on making this more efficient.
My header file:
My C source code:
```
/**
* @file json.c
* @brief JSON Parser
*/
#include
#include "json.h"
/**
* @fn s
My header file:
#ifndef _json_H_
#define _json_H_
/**
* JSON type identifier. Basic types are:
* o Object
* o Array
* o String
* o Other primitive: number, boolean (true/false) or null
*/
typedef enum { JSON_PRIMITIVE, JSON_OBJECT, JSON_ARRAY, JSON_STRING } JsonType;
typedef enum
{
JSON_ERROR_NOMEM = -1, // Not enough tokens were provided
JSON_ERROR_INVAL = -2, // Invalid character inside JSON string
JSON_ERROR_PART = -3, // The string is not a full JSON packet, more bytes expected
JSON_SUCCESS = 0 // Everthing is fine
} JsonError;
/**
* JSON token description.
* @param type type (object, array, string etc.)
* @param start start position in JSON data string
* @param end end position in JSON data string
*/
typedef struct
{
JsonType type;
int start;
int end;
int size;
#ifdef json_PARENT_LINKS
int parent;
#endif
} JsonToken;
/**
* JSON parser. Contains an array of token blocks available. Also stores
* the string being parsed now and current position in that string
*/
typedef struct
{
unsigned int pos; /* offset in the JSON string */
unsigned int toknext; /* next token to allocate */
int toksuper; /* superior token node, e.g parent object or array */
} JsonParser;
/**
* Create JSON parser over an array of tokens
*/
void json_initJsonParser(JsonParser *parser);
/**
* Run JSON parser. It parses a JSON data string into and array of tokens, each describing
* a single JSON object.
*/
JsonError json_parseJson(JsonParser *parser, const char *js, JsonToken *tokens, unsigned int tokenNum);
#endif /* _json_H_ */My C source code:
```
/**
* @file json.c
* @brief JSON Parser
*/
#include
#include "json.h"
/**
* @fn s
Solution
What you did well
What you could improve on
-
It is not obvious that the way to extract the results is to use a loop like
Please feel free to add some generous comments with usage examples in the header file.
- As far as I can tell, the code produces correct output when given correct input.
- The functions feel like a coherent library, with consistent function names and logical order of parameters.
- You use a caller-owns-everything memory management policy, which works well for C.
What you could improve on
- The name
json_initJsonParser()is a bit redundant.json_initParser()should suffice.
- I don't think that
#ifdef json_PARENT_LINKSis a good idea. A user could easily compile the library without support for parent links, yet compile the application withjson_PARENT_LINKSdefined, leading to nasty bugs. Why not always enable support parent links? It would be pretty difficult to make sense of the parser output without it.
- I think that
#ifdef json_STRICTwould be better as a runtime option rather than a compile-time option. If the user wants to have both modes available, there's no reasonable way to do so.
- JSON primitives may be numbers,
true,false, ornull. You accept illegal primitives such astruthy.
json_allocJsonToken()doesn't "allocate" memory in the sense that I expect. Also, two of the three calls tojson_allocJsonToken()are followed byjson_fillToken(), so you might as well combine the two functions. Filling in dummy values first is senseless.
- In the
JsonParserstructure,toknextandtoksupershould be the same data type. It's weird that one is unsigned and the other is signed. I recommend just usingintfor both, or possiblyssize_t.
-
It is not obvious that the way to extract the results is to use a loop like
for (int i = 0; i < parser.toknext; i++) {
// ^^^^^^^^^^^^^^
}Please feel free to add some generous comments with usage examples in the header file.
Code Snippets
for (int i = 0; i < parser.toknext; i++) {
// ^^^^^^^^^^^^^^
}Context
StackExchange Code Review Q#43872, answer score: 13
Revisions (0)
No revisions yet.