From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 26 Jun 2014 19:00:01 GMT
Subject: Re: [PATCH v3 2/3] config: add hashtable for config parsing & retrieval
Message-ID: <xmqqpphv4hu6.fsf@gitster.dls.corp.google.com>
In-Reply-To: <vpq1tubinbx.fsf@anie.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> When the submodule script that uses "git config -f .gitmodules" is
>> converted into C, if the updated config API is ready, it may be able
>> to do something like these in a single program:
>>
>> 	const char *url;
>> 	struct config_set *gm_config;
>>
>>         /* read from $GIT_DIR/config */
>>         url = git_config_get_string("remote.origin.url");
>>
>>         /*
>>          * allow us to read from .gitmodules; the parameters are
>>          * list of files that make up the configset, perhaps.
>>          */
>> 	gm_config = git_configset_new(".gitmodules", NULL);
>>
>>
>>         if (!git_configset_get_bool(gm_config, "submodule.frotz.ignore")) {
>> 		/* do a lot of stuff for the submodule */
>>                 ...
>> 	}
>>
>>         /* when we are done with the configset */
>>         git_configset_clear(gm_config);
>
> Isn't that a bit overkill? Why not just let the caller manage a hashmap
> directly instead of a config_set?

Because I had an experience under my belt of a painful refactoring
of "the_index" which turned out to be not just a single array, I
simply suspect that the final data structure to represent a "set of
config-like things" will not be just a single hashmap, hence I do
prefer to have one layer of abstraction "struct config_set", which
would contain a hashmap and possibly more.  Doesn't "is the hashmap
initialized" bit belong there, for example?