Re: [PATCH v5 2/3] advice: revamp advise API

[Heba Waly <heba.waly@gmail.com>]

On Wed, Feb 26, 2020 at 4:03 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Heba Waly <heba.waly@gmail.com> writes:
>
> > I'm not against this approach as well, but as I mentioned above, we
> > need a list of keys to be returned by list_config_advices(), that's
> > why defining the constant strings will not be sufficient in our case.
>
> Sorry, but I do not get it.
>
> Either you use enum or a bunch of variables of type const char [],
> "list all of them" would need an array whose elements are all of
> them, so
>
>         const char ADVICE_FOO[] = "advice.foo";
>         const char ADVICE_BAR[] = "advice.bar";
>         ...
>
>         static const char *all_advice_type[] = {
>                 ADVICE_FOO, ADVICE_BAR, ...
>         };
>
>         void for_each_advice_type(int (*fn)(const char *name))
>         {
>                 int i;
>                 for (i = 0; i < ARRAY_SIZE(all_advice_type); i++)
>                         fn(all_advice_type[i]);
>         }
>
> would be sufficient, and I do not think it takes any more effort to
> create and manage than using an array indexed with the enum, no?
>

hmm, you're right, I just personally prefer having related variables
collected in one data structure (whenever possible) like a list (or
enum in this case) rather than defining each independently as a const
variable. On the other hand, I understand that you'd prefer to skip
the extra step of converting the enum to string.
hmmm ok, I'll change the enum and send a new version soon.

Thanks,
Heba