From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id D0F4643D67A for ; Sat, 19 Oct 2019 18:03:37 +0300 (MSK) References: <20190903192433.GF15611@atlas> <20191005132231.GE3913@atlas> <20191005135014.GI3913@atlas> From: Vladislav Shpilevoy Message-ID: <33fd3f27-8a35-4d07-6da4-9bd7cd072949@tarantool.org> Date: Sat, 19 Oct 2019 17:08:48 +0200 MIME-Version: 1.0 In-Reply-To: <20191005135014.GI3913@atlas> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH v2 3/8] json: lexer_eof and token_cmp helper functions List-Id: Tarantool development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konstantin Osipov Cc: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org Hi! Thanks for the review! On 05/10/2019 15:50, Konstantin Osipov wrote: > * Vladislav Shpilevoy [19/09/28 09:11]: >>>> /** >>>> - * Compare JSON token keys. >>>> + * Compare JSON tokens as nodes of a JSON tree. That is, including >>>> + * parent references. >>>> */ >>> >>> Please explain how it is useful. Why not json_tree_node_cmp then? >> >> What is useful? Being able to compare two JSON nodes? Not sure if I >> understand the question. I just need to compare two nodes. In several >> places. And I added a function for that. > > How it is independently useful , given there still exists the old cmp function. > You added a (yet) another cmp function, the comment should contain > the usage scenario for it. > Sorry, but it already looks like hypergraphia. This function's body consists of 12 the simplest possible lines, including braces. Its purpose is obvious. Just compare two tokens and return something that is < 0, == 0, or > 0. What a usage scenario can there be except comparison of two tokens? You are saying, that all the code should be commented, each function should be described, even a trivial one, but now I can definitely say, that it is not necessary, and does not make life much easier. I am working in a project, that is way bigger than Tarantool, and comments are written for exceptional cases only, where code is not trivial. And this does not harm at all. When code is easy to understand, comments won't help to understand it even better. Moreover, they may confuse. They become comments just for comments. Here is the diff: ======================================================================== diff --git a/src/lib/json/json.h b/src/lib/json/json.h index 08a0ee96c..3218769a1 100644 --- a/src/lib/json/json.h +++ b/src/lib/json/json.h @@ -288,7 +288,10 @@ json_token_is_leaf(struct json_token *token) /** * Compare two JSON tokens, not taking into account their tree - * attributes. + * attributes. Only the token values are compared. That might be + * used to compare two JSON paths. String comparison of the paths + * may not work because the same token can be present in different + * forms: ['a'] == .a, for example. */ static inline int json_token_cmp(const struct json_token *l, const struct json_token *r)