Re: [PATCH 0/11] renaming argv_array

From: Christian Couder <christian.couder@gmail.com>
Date: Jul 29, 2020, 06:15 UTC
Message-ID: <CAP8UFD26J6W50SeQqJiG0y04kcdNzr6RRT7ZeJmrQ2V-QTS3Lg@mail.gmail.com>
In-Reply-To: <20200728202124.GA1021264@coredump.intra.peff.net>

On Tue, Jul 28, 2020 at 10:23 PM Jeff King <peff@peff.net> wrote:

Show 8 quoted lines

>
> The argv_array data type has turned out to be useful in our code base,
> but the name isn't very good. From patch 2 of this series:
>
>   The name "argv-array" isn't very good, because it describes what the
>   data type can be used for (program argument arrays), not what it
>   actually is (a dynamically-growing string array that maintains a
>   NULL-terminator invariant).

I cannot help but notice that you still use "array" when describing what it is. You actually use "string array" to describe what it is, and at the same time say that the name should describe what it is. So I would expect after the above sentence that you would rename it to "string_array" or "str_array".

In patch 2 you also say:

> I settled on "strvec" because "vector" is the name for a dynamic array
> type in many programming languages.

in which you also use "array" to describe it.

> "strarray" would work, too, but it's
> longer and a bit more awkward to say (and don't we all say these things
> in our mind as we type them?).

It's longer than "strarray" by 2 characters only.

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.

So if you don't like the "array" part of the name, are you going to also change "oid_array" into "oidvec" and for example "REALLOC_ARRAY" into "REALLOC_VEC" or "REALLOC_VECTOR"?

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.