From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Kirill Shcherbatov Subject: Re: [tarantool-patches] Re: [PATCH v5 05/12] lib: implement JSON tree class for json library References: <20181101150854.GG30032@chai> Message-ID: Date: Tue, 6 Nov 2018 15:15:08 +0300 MIME-Version: 1.0 In-Reply-To: <20181101150854.GG30032@chai> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit To: tarantool-patches@freelists.org, Kostya Osipov Cc: Vladimir Davydov List-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. 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. */