From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 25 Feb 2020 20:09:12 GMT
Subject: Re: [PATCH v5 2/3] advice: revamp advise API
Message-ID: <xmqqblpmmntj.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20200225195648.GA212281@google.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.