All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: Jeff King <peff@peff.net>, git <git@vger.kernel.org>
Subject: Re: [PATCH 0/11] renaming argv_array
Date: Wed, 29 Jul 2020 09:32:36 -0400	[thread overview]
Message-ID: <CAPig+cQEqhpu3-_wXUFjfokSuesu2a5QdiaT8ks4kPiYkUsOvw@mail.gmail.com> (raw)
In-Reply-To: <CAP8UFD26J6W50SeQqJiG0y04kcdNzr6RRT7ZeJmrQ2V-QTS3Lg@mail.gmail.com>

On Wed, Jul 29, 2020 at 2:16 AM Christian Couder
<christian.couder@gmail.com> wrote:
> I would expect after the above sentence that you would rename it to
> "string_array" or "str_array".
> [...]
> Also we still use "array" in "oid_array" which is very similar to
> this. And the implementation is based on the ALLOC_GROW macro which
> uses the REALLOC_ARRAY macro.
>
> We also use ALLOC_ARRAY, FLEX_ARRAY, CALLOC_ARRAY, COPY_ARRAY and
> MOVE_ARRAY macros.
> [...]
> If you want to change only "argv_array" (and not also "oid_array",
> "REALLOC_ARRAY" and perhaps other *_ARRAY macros) into something else,
> then I think it would be better to be consistent with them.

Dipping my toe into the bikeshed paint bucket...

Naming consistency is certainly a valid concern, and if this was a
public API I might agree that such consistency should outweigh some
other concerns, however, this is a private, project-specific API in
which convenience and concision weigh more heavily in my opinion.
There is value in succinctness, not just when writing code (by having
to type less), but also when reading it, for the same reason that we
use short and sweet variable and function names. To wit: short, well
chosen, idiomatic names let the structure of the code show through
(often) at-a-glance, whereas long names force us to laboriously read
code, making it harder to discern the overall structure and logic.

There is also value[1] in having "vec" (or "v") in the name as opposed
to "array", specifically because this isn't just an array of strings;
it is a NULL-terminated vector of strings. Thus, it is suitable for
functions which accept such a datatype, which often have "v" in their
names, as well (for instance, execv()).

On the topic of a "terminated array": As a NULL-terminated array of
strings, the name "strvec" provides naming consistency and fits nicely
alongside the name "strbuf", a NUL-terminated array of characters,
while also sharing the relative concision of that name.

In my opinion, the name "strvec" is a good and reasonably succinct
compromise between other names, such as "strv"[2] and "strs"[3],
proposed previously. It gets my "+1".

[1]: https://lore.kernel.org/git/20180227221808.GE11187@sigill.intra.peff.net/
[2]: https://lore.kernel.org/git/20180227220443.GB11187@sigill.intra.peff.net/
[3]: https://lore.kernel.org/git/CAPig+cS+G-xC51n-Ud0Wbmcc-zeHBM3-5WQQAFm9gwm9LNk3Gg@mail.gmail.com/

  parent reply	other threads:[~2020-07-29 13:32 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-28 20:21 Jeff King
2020-07-28 20:21 ` [PATCH 01/11] argv-array: use size_t for count and alloc Jeff King
2020-07-28 20:23 ` [PATCH 02/11] argv-array: rename to strvec Jeff King
2020-07-28 20:23 ` [PATCH 03/11] strvec: rename files from argv-array " Jeff King
2020-07-28 20:24 ` [PATCH 04/11] quote: rename sq_dequote_to_argv_array to mention strvec Jeff King
2020-07-28 20:24 ` [PATCH 05/11] strvec: convert builtin/ callers away from argv_array name Jeff King
2020-07-28 20:24 ` [PATCH 06/11] strvec: convert more " Jeff King
2020-07-28 20:25 ` [PATCH 07/11] strvec: convert remaining " Jeff King
2020-07-28 20:26 ` [PATCH 08/11] strvec: fix indentation in renamed calls Jeff King
2020-07-28 22:43   ` Jacob Keller
2020-07-28 23:31     ` Junio C Hamano
2020-07-28 20:26 ` [PATCH 09/11] strvec: update documention to avoid argv_array Jeff King
2020-07-28 20:27 ` [PATCH 10/11] strvec: drop argv_array compatibility layer Jeff King
2020-07-28 22:23   ` Junio C Hamano
2020-07-29  0:04     ` Jeff King
2020-07-29  0:37       ` Jeff King
2020-07-29  0:40         ` Jeff King
2020-07-29  0:47           ` Junio C Hamano
2020-07-29 16:54             ` Derrick Stolee
2020-07-29  0:44         ` Junio C Hamano
2020-07-29 16:22           ` Jeff King
2020-07-28 20:28 ` [PATCH 11/11] strvec: rename struct fields Jeff King
2020-07-28 21:16   ` Junio C Hamano
2020-07-28 21:18     ` Junio C Hamano
2020-07-29  6:55       ` Christian Couder
2020-07-29 16:34         ` Jeff King
2020-07-29 18:03           ` Junio C Hamano
2020-07-28 21:20     ` Jeff King
2020-07-28 22:45 ` [PATCH 0/11] renaming argv_array Jacob Keller
2020-07-29  0:06   ` Jeff King
2020-07-29  6:15 ` Christian Couder
2020-07-29  6:19   ` Christian Couder
2020-07-29 13:32   ` Eric Sunshine [this message]
2020-07-29 16:33   ` Jeff King
2020-08-11 16:08 ` René Scharfe
2020-08-11 18:28   ` Taylor Blau
2020-08-11 19:00   ` Junio C Hamano
2020-08-11 20:39     ` Jacob Keller
2020-08-11 21:03       ` Junio C Hamano
2020-08-12 12:42     ` Johannes Schindelin
2020-08-12 15:06   ` Jeff King
2020-08-12 15:10     ` Jeff King
2020-08-12 16:23       ` René Scharfe
2020-08-12 17:08         ` Jeff King
2020-08-12 18:18           ` René Scharfe
2020-08-12 19:57             ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPig+cQEqhpu3-_wXUFjfokSuesu2a5QdiaT8ks4kPiYkUsOvw@mail.gmail.com \
    --to=sunshine@sunshineco.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --subject='Re: [PATCH 0/11] renaming argv_array' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.