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

[Junio C Hamano <gitster@pobox.com>]

Emily Shaffer <emilyshaffer@google.com> writes:

>> ...  Your get_config_value() would then become a mere lookup
>> in advice_setting[] array, e.g.
>> 
>> 	int advice_enabled(unsigned advice_type)
>> 	{
>> 		static int initialized;
>> 
>> 		if (!initialized) {
>> 			initialized = 1;
>> 			git_config(populate_advice_settings, NULL);
>> 		}
>> 		if (ARRAY_SIZE(advice_setting) <= advice_type)
>> 			BUG("OOB advice type requested???");
>> 		return !advice_setting[advice_type].disabled;
>> 	}
>> 
>> with your "push-update-rejected has two names" twist added.
>
> I'm a little confused about the need to cache the result of
> git_config_get_bool() - isn't that a lookup from a hashmap which is
> already populated at setup time, and therefore inexpensive?

Looking up from hashmap with a string key is always more expensive
than indexing into a linear array with the array index.  Also, the
suggested arrangement makes the advice API implementation more self
contained, I'd think.