[tarantool-patches] Re: [PATCH v5 05/12] lib: implement JSON tree class for json library

[Kirill Shcherbatov]

> This is hard to read. It's hard to see whether it's a hash lookup
> or a tree lookup. Let's perhaps rename mh_node_t to mh_entry_t.
Ok, done.

> Even more confusing is json_path_node and json_tree_node. 
> Let's take time to come up with better names.
Maybe the "json_tree_entry" is a better name for JSON tree record structure?
As for json_tree_node, this is an existent class that I don't touch in my patch at all.

>> +json_tree_lookup_by_path(struct json_tree *tree, struct json_tree_node *parent,
>> +			 const char *path, uint32_t path_len)
> This function could be called simply json_tree_lookup().
Done. 
>> +	uint32_t arr_size = parent->children_count;
> 
> child_count.
Ok.

> 
>> +struct json_tree_node *
>> +json_tree_next_pre(struct json_tree_node *parent, struct json_tree_node *pos)
> 
> I would call it preorder/postorder to avoid confusion between pre
> and prev.
Ok, don't mind.

> json_tree_postorder_next
Ok, same for json_tree_preorder_next

> 
>> +/** Compute the hash value of a JSON path component. */
>> +uint32_t
>> +json_path_node_hash(struct json_path_node *key, uint32_t seed);
> 
> This is json_path_fragment or json_path_segment.
uint32_t
json_path_fragment_hash(struct json_path_node *key, uint32_t seed)
> What is a node item? I'm in hell./**
 * Return container entry by json_tree_entry item or NULL if
 * item is NULL.
 */