From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <alexander.turenko@tarantool.org>
Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39])
(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 82F76469719
for <tarantool-patches@dev.tarantool.org>;
Fri, 9 Oct 2020 04:10:46 +0300 (MSK)
Date: Fri, 9 Oct 2020 04:11:02 +0300
From: Alexander Turenko <alexander.turenko@tarantool.org>
Message-ID: <20201009011102.idolx3rnrk67mt43@tkn_work_nb>
References: <cover.1600955781.git.tsafin@tarantool.org>
<e54cb665e54d3233024e625f2c12c0625bf5468a.1600955781.git.tsafin@tarantool.org>
<8f0dfb00-0dbe-6717-1c36-90957a072751@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <8f0dfb00-0dbe-6717-1c36-90957a072751@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH 2.X 1/7] module api: export
box_tuple_validate
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>,
<mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>,
<mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
> > +int
> > +box_tuple_validate(box_tuple_format_t *format, box_tuple_t *tuple);
>
> 2. OCD mode on. I would propose either make tuple the first
> argument, or rename it to box_tuple_format_validate_tuple().
> So as to be consistent with our agreement, that if something
> is a method of <type>, then the <type> argument goes first,
> and the method name is <type>_<action>.
>
> I see we currently have in the public API the functions:
>
> box_tuple_validate - your new function, a bit
> inconsistent.
>
> box_tuple_validate_key_parts - this should have been
> box_key_def_validate_tuple from the beginning,
> but we can't do anything about it now.
We can. It is part of my patchset.
>
> box_key_def_validate_key - correct. Key_def goes first,
> and the name is consistent.
>
> So if you will make box_tuple_validate consistent, we will have
> more correct signatures (2/3) than incorrect, for validation
> methods at least.
So, if we'll apply all your suggestions, the key_def module API will
contain the following functions:
| Function | Consumer | Name variants (for history) |
| ---------------------------- | --------------- | ------------------------------- |
| box_key_def_new() | already present | |
| box_key_part_def_create() | key_def module | |
| box_key_def_new_v2() | key_def module | box_key_def_new_ex() |
| box_key_def_dump_parts() | key_def module | |
| box_key_def_merge() | key_def module | |
| box_key_def_dup() | merger module | |
| box_key_def_delete() | already present | |
| box_key_def_validate_tuple() | key_def module | box_tuple_validate_key_parts() |
| box_tuple_compare() | already present | |
| box_tuple_compare_with_key() | already present | |
| box_key_def_extract_key() | key_def module | box_tuple_extract_key_{ex,v2}() |
| box_key_def_validate_key() | key_def module | |
All functions around key_defs and tuples are prefixed with 'box_key_def_',
except box_tuple_compare*(), which are already present.
If we'll follow current internal naming:
| Function | Name variants (may fit better) |
| ------------------------------ | ------------------------------ |
| box_key_def_new() | |
| box_key_part_def_create() | |
| box_key_def_new_v2() | |
| box_key_def_dump_parts() | |
| box_key_def_merge() | |
| box_key_def_dup() | |
| box_key_def_delete() | |
| box_tuple_validate_key_parts() | box_tuple_validate_key() |
| box_tuple_compare() | |
| box_tuple_compare_with_key() | |
| box_tuple_extract_key_v2() | |
| box_key_def_validate_key() | box_validate_key() |
Here functions that operate on key_def itself are prefixed with
'box_key_def_', but functions that operate on tuples using a key
definition are named 'box_tuple_<action>()' (generally, see below).
The exception is box_key_def_validate_key(), but we can rename it to
box_validate_key(). And also drop '_parts' from
box_tuple_validate_key_parts() (because it meaningless):
| Function |
| ------------------------------ |
| box_key_def_new() |
| box_key_part_def_create() |
| box_key_def_new_v2() |
| box_key_def_dump_parts() |
| box_key_def_merge() |
| box_key_def_dup() |
| box_key_def_delete() |
| box_tuple_validate_key() |
| box_tuple_compare() |
| box_tuple_compare_with_key() |
| box_tuple_extract_key_v2() |
| box_validate_key() |
Isn't that nice?