Re: [PATCH 0/11] renaming argv_array

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 29, 2020, 13:32 UTC
Message-ID: <CAPig+cQEqhpu3-_wXUFjfokSuesu2a5QdiaT8ks4kPiYkUsOvw@mail.gmail.com>
In-Reply-To: <CAP8UFD26J6W50SeQqJiG0y04kcdNzr6RRT7ZeJmrQ2V-QTS3Lg@mail.gmail.com>

On Wed, Jul 29, 2020 at 2:16 AM Christian Couder <christian.couder@gmail.com> wrote:

Show 13 quoted lines

> 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/