HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

More efficient way to retrieve data from JSON

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
morewayefficientretrievejsonfromdata

Problem

I am using the jsmn JSON parser (source code) to get extract data from a JSON string. jsmn stores the data in tokens that just point to the token boundaries in the JSON string instead of copying the data. For example, jsmn will create tokens like:

-
Object [0..31]

-
String [3..7], String [12..16], String [20..23]

-
Number [27..29]

This method is used to retrieve the actual characters between those values (for String objects):

char* getTextFromJSON(const char *json)
{
    json_parser p;
    jsontok_t tokens[15];
    jsontok_t key;

    if (!json) return NULL;

    initJsonParser(&p);
    parseJson(&p, json, tokens, sizeof(tokens) / sizeof(tokens[0]));
    for(unsigned int i = 0; i < sizeof(tokens) / sizeof(tokens[0]); ++i)
    {
        key = tokens[i];
        unsigned int length = key.end - key.start;
        char keyString[length + 1];
        memcpy(keyString, &json[key.start], length);
        keyString[length] = '\0';
        if(!strcmp(keyString, "utterance"))
        {
            key = tokens[i + 1];
            length = key.end - key.start;
            char* keyString = (char*) malloc(length + 1);
            memcpy(keyString, &json[key.start], length);
            keyString[length] = '\0';
            return keyString;
        }
    }
    return NULL;
}


Here are some JSON examples that would be thrown into the parser:

{"status":0,"id":"432eac38858968c108899cc6c3a4bade-1","hypotheses" [{"utterance":"test","confidence":0.84134156}]}


{"status":5,"id":"695118aaa3d01dc2ac4aa8054d1e5bb0-1","hypotheses":[]}

Solution

There are a few issues with getTextFromJSON involving use of pointers, code
duplication and use of an inappropriate function.

Pointers first. In this code:

jsontok_t tokens[15];
jsontok_t key;
...
key = tokens[i];


you are copying the token when you could be using a pointer:

jsontok_t *key = &tokens[i];


Copying is obviously more costly than pointing. Also note that it is often
best to define the variable at its first point of use.

There are two points of duplication, one determining the size of the token
array and one concerned with copying the string. Instead of this:

jsontok_t tokens[15];
...
parseJson(&p, json, tokens, sizeof(tokens) / sizeof(tokens[0]));
for(unsigned int i = 0; i < sizeof(tokens) / sizeof(tokens[0]); ++i)


I would do either:

#define N_TOKENS 15
jsontok_t tokens[N_TOKENS];
...
parseJson(&p, json, tokens, N_TOKENS);
for(unsigned int i = 0; i < N_TOKENS; ++i)


or

jsontok_t tokens[15];
#define N_TOKENS (sizeof tokens / sizeof tokens[0])  // or const int N_TOKENS inseatd of #define 
...etc


The string copying code-duplication is actually unnecessary if you use a
better standard library function...

Instead of using strcmp to compare the string (which involves constructing
the string because it lacks a \0):

if(!strcmp(keyString, "utterance"))


use memcmp. And you can use strndup to create a copy of the key. If you
don't have strndup which is non-standard, you can easily write it (in a
separate function).

These changes wont speed it up much, but are certainly cleaner:

char* getTextFromJSON(const char *json)
{
    if (!json) return NULL;

    json_parser p;
    #define N_TOKENS 15  // this normally would be at the start of the file
    jsontok_t tokens[N_TOKENS];

    initJsonParser(&p);
    int n = parseJson(&p, json, tokens, N_TOKENS);
    for (int i = 0; i start], (size_t) (key->end - key->start))) {
            ++key;
            return strndup(&json[key->start], (size_t)(key->end - key->start));
        }
    }
    return NULL;
}

Code Snippets

jsontok_t tokens[15];
jsontok_t key;
...
key = tokens[i];
jsontok_t *key = &tokens[i];
jsontok_t tokens[15];
...
parseJson(&p, json, tokens, sizeof(tokens) / sizeof(tokens[0]));
for(unsigned int i = 0; i < sizeof(tokens) / sizeof(tokens[0]); ++i)
#define N_TOKENS 15
jsontok_t tokens[N_TOKENS];
...
parseJson(&p, json, tokens, N_TOKENS);
for(unsigned int i = 0; i < N_TOKENS; ++i)
jsontok_t tokens[15];
#define N_TOKENS (sizeof tokens / sizeof tokens[0])  // or const int N_TOKENS inseatd of #define 
...etc

Context

StackExchange Code Review Q#28986, answer score: 8

Revisions (0)

No revisions yet.