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

[Konstantin Osipov <kostja@tarantool.org>]

* Kirill Shcherbatov <kshcherbatov@tarantool.org> [18/10/29 20:25]:
> +	mh_int_t id = mh_json_tree_node_find(tree->hash, &info, NULL);
> +	if (unlikely(id == mh_end(tree->hash)))
> +		return NULL;
> +	struct mh_json_tree_node *ht_node =
> +		mh_json_tree_node_node(tree->hash, id);

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.

Even more confusing is json_path_node and json_tree_node. 
Let's take time to come up with better names.


> +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().

> +	struct json_tree_node **arr = parent->children;
> +	uint32_t arr_size = parent->children_count;

child_count.

> +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.

> +struct json_tree_node *
> +json_tree_next_post(struct json_tree_node *parent, struct json_tree_node *pos)

json_tree_postorder_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.

> +#define json_tree_entry(item, type, member) ({				     \
> +	const typeof( ((type *)0)->member ) *__mptr = (item);		     \
> +	(type *)( (char *)__mptr - ((size_t) &((type *)0)->member) ); })

> +
> +/** Return entry by json_tree_node item or NULL if item is NULL.*/
> +#define json_tree_entry_safe(item, type, member) ({			     \
> +	(item) != NULL ? json_tree_entry((item), type, member) : NULL; })

What is a node item? I'm in hell.

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov