snippetcMinor
Parsing my JSON-like format in C
Viewed 0 times
jsonformatparsinglike
Problem
For the past few weeks, I've been using the wonderful cJSON to add content to my game. But recently, I got tired of JSON's verbosity so I decided to roll my own format called SON (Simple Object Notation). I plan on using this format in all of my future game projects.
My parser is closely modeled after cJSON. I still have work to do (search by key, file writing, etc) but I'd like to get feedback on what I have so far. The three things I'm concerned about are:
sonparse.h
```
#pragma once
/ A node in a son file with accompanying value and relatives /
typedef struct son son;
struct son {
char* key;
son* child;
son* next;
enum { SON_NONE, SON_INTEGER, SON_STRING, SON_ARRAY } type;
union {
int integer;
char* string;
struct {
int* data;
int size;
} array;
};
};
/ Create a new node and set pointers to NULL. /
son* son_new(void);
/ Recursively free a node and all of its relatives. /
v
rogue
anim
timer 0
brain
type player
image
visibility visible
texture rogue
imagebox 0 0 24 40
imageoff 0 -20
player
pos
loc -30 85
dir south
solidity solid
kobold
anim
timer 0
brain
type monster
image
visibility visible
texture kobold
imagebox 0 0 24 40
imageoff 0 -20
pos
loc 12 45
dir south
solidity solidMy parser is closely modeled after cJSON. I still have work to do (search by key, file writing, etc) but I'd like to get feedback on what I have so far. The three things I'm concerned about are:
- How is my coding style? I've only been using C for a few weeks and I'm still trying to find what works for me. I used to write C++ capitalization in Allman but I've recently switched to C-style capitalization and 1TBS.
- Is my file parsing efficient? This is my first time reading files in C and I struggled for several days figuring out the recursion part.
- How would approach writing my SON tree back to a file? What algorithms do I need to learn? I want my output to look like the input character-for-character.
sonparse.h
```
#pragma once
/ A node in a son file with accompanying value and relatives /
typedef struct son son;
struct son {
char* key;
son* child;
son* next;
enum { SON_NONE, SON_INTEGER, SON_STRING, SON_ARRAY } type;
union {
int integer;
char* string;
struct {
int* data;
int size;
} array;
};
};
/ Create a new node and set pointers to NULL. /
son* son_new(void);
/ Recursively free a node and all of its relatives. /
v
Solution
Welcome to Code Review, nice first question. If you had more points I would
suggest putting a bounty on the question to get more answers.
Possible Bugs
The first possible bug is in
A good practice is to initialize all fields in an allocated structure, the previous value of the memory location is unknown.
The next possible bug is in
Consider using
One possible bug in the preceding code is that it may try to convert
non-numeric characters, there should at least be a check using
A Second possible bug in the preceding code is that the numeric value of the
converted string may be larger than the word size on the computer running the
program.
Coding Style
This question is somewhat subjective, you will usually get some comments or
answers here on Code Review if the style is bad or can be improved. As long
as you pick a coding convention such as
with it throughout the code you're doing good. Your indentation is already
fine. Some people may have avoided answering the question because of the
subjectivity of the question.
If you are coding professionally the company or the department may have
coding standards that you must use (it makes it easier for other developers
to fix the code when you are on vacation or have gotten a better job).
Naming Conventions for Types, Functions and Variables
If the functions were more descriptively named, the need for comments would
be greatly reduced. Comments are generally used to describe what the
the algorithm is or why the specific algorithm was used. Using underscores
in a name does differentiate the word in the name but can take up a lot
more space. The Example Usage comment in
what a comment should be. The comment
Examples:
Typedef's
The code in son_parse.h could be made more readable in this manner:
For more object oriented pointers to functions in the struct can be added.
Some pointers to functions that might be added are:
This would remove the necessity for the declarations of
and
Debug Code
Embed the debug code in
Example
in son_parse.h:
in son_parse.c
You can define DEBUG in the CC or gcc command line, have a DEBUG target in
a make file, or your IDE may provide a DEBUG mode that defines the DEBUG
macro for you.
Global Variable
suggest putting a bounty on the question to get more answers.
Possible Bugs
The first possible bug is in
son_new(), key is not initialized to NULL.A good practice is to initialize all fields in an allocated structure, the previous value of the memory location is unknown.
The next possible bug is in
son_parse(), the variable file is not checked to see if a it contains a valid file pointer. If fopen() fails the value of file will be NULL, and any use of the file pointer will result in a program crash. The Global Variable errno will provide an indication of why the call failed. The fread() and fwrite() functions can both fail.Consider using
intatoi(const char* string) rather than using the following code in read_array():c = getc(file);
while (c && c != '\n') {
if (c == '-') {
sign = -1;
} else if (c == ' ') {
++i;
sign = 1;
node->array.data[i] = 0;
} else {
/* Add digit to current index. */
node->array.data[i] *= 10;
node->array.data[i] += (c - '0') * sign;
}
c = getc(file);
}One possible bug in the preceding code is that it may try to convert
non-numeric characters, there should at least be a check using
isdigit(char c).A Second possible bug in the preceding code is that the numeric value of the
converted string may be larger than the word size on the computer running the
program.
Coding Style
This question is somewhat subjective, you will usually get some comments or
answers here on Code Review if the style is bad or can be improved. As long
as you pick a coding convention such as
camelCase or Capitalization and stickwith it throughout the code you're doing good. Your indentation is already
fine. Some people may have avoided answering the question because of the
subjectivity of the question.
If you are coding professionally the company or the department may have
coding standards that you must use (it makes it easier for other developers
to fix the code when you are on vacation or have gotten a better job).
Naming Conventions for Types, Functions and Variables
If the functions were more descriptively named, the need for comments would
be greatly reduced. Comments are generally used to describe what the
the algorithm is or why the specific algorithm was used. Using underscores
in a name does differentiate the word in the name but can take up a lot
more space. The Example Usage comment in
son_parse is a good example ofwhat a comment should be. The comment
/ Check if a node was returned to this depth / is also a good comment.Examples:
- The struct
sonmight be better namedson_node, orSonNode.
- The
son_newfunction might be better namedson_new_nodeor
SonNewNode.- The file variable in son_parse.c might be better named
SonInputFile().Typedef's
The code in son_parse.h could be made more readable in this manner:
typedef enum { SON_NONE, SON_INTEGER, SON_STRING, SON_ARRAY } SonType;
typedef struct son_node {
char* key;
son* child;
son* next;
SonType type;
union {
int integer;
char* string;
struct {
int* data;
int size;
} array;
};
} SonNode;For more object oriented pointers to functions in the struct can be added.
Some pointers to functions that might be added are:
AddChild()
DeleteChild()
AddNextNode()
DeleteNextNode()
WriteNodeToOutputFile()
PrintNode()
This would remove the necessity for the declarations of
son_new(), son_del()and
son_print() in son_parse.hDebug Code
Embed the debug code in
#ifdef DEBUGExample
in son_parse.h:
#ifdef DEBUG
void son_print();
#endifin son_parse.c
#ifdef DEBUG
void son_print(son* node) {
printf("%s", node->key);
/* Print node value, or (null) if none. */
char* valuestr = NULL;
switch (node->type) {
case SON_INTEGER:
printf(" : %d\n", node->integer);
break;
case SON_ARRAY:
printf(" :");
for(int i = 0; i array.size; ++i) {
printf(" %d", node->array.data[i]);
}
putchar('\n');
break;
case SON_STRING:
valuestr = node->string;
case SON_NONE:
default:
printf(" : %s\n", valuestr);
break;
}
/* Print node siblings, or (null) if none. */
char* nextstr = NULL;
char* childstr = NULL;
if (node->next) {
nextstr = node->next->key;
}
if (node->child) {
childstr = node->child->key;
}
printf("next: %s child: %s\n", nextstr, childstr);
}
#endifYou can define DEBUG in the CC or gcc command line, have a DEBUG target in
a make file, or your IDE may provide a DEBUG mode that defines the DEBUG
macro for you.
Global Variable
Code Snippets
c = getc(file);
while (c && c != '\n') {
if (c == '-') {
sign = -1;
} else if (c == ' ') {
++i;
sign = 1;
node->array.data[i] = 0;
} else {
/* Add digit to current index. */
node->array.data[i] *= 10;
node->array.data[i] += (c - '0') * sign;
}
c = getc(file);
}typedef enum { SON_NONE, SON_INTEGER, SON_STRING, SON_ARRAY } SonType;
typedef struct son_node {
char* key;
son* child;
son* next;
SonType type;
union {
int integer;
char* string;
struct {
int* data;
int size;
} array;
};
} SonNode;#ifdef DEBUG
void son_print();
#endif#ifdef DEBUG
void son_print(son* node) {
printf("%s", node->key);
/* Print node value, or (null) if none. */
char* valuestr = NULL;
switch (node->type) {
case SON_INTEGER:
printf(" : %d\n", node->integer);
break;
case SON_ARRAY:
printf(" :");
for(int i = 0; i < node->array.size; ++i) {
printf(" %d", node->array.data[i]);
}
putchar('\n');
break;
case SON_STRING:
valuestr = node->string;
case SON_NONE:
default:
printf(" : %s\n", valuestr);
break;
}
/* Print node siblings, or (null) if none. */
char* nextstr = NULL;
char* childstr = NULL;
if (node->next) {
nextstr = node->next->key;
}
if (node->child) {
childstr = node->child->key;
}
printf("next: %s child: %s\n", nextstr, childstr);
}
#endif#pragma onceContext
StackExchange Code Review Q#149466, answer score: 5
Revisions (0)
No revisions yet.