Re: [PATCH v3] environment: move access to "core.sparsecheckout" into repo_settings

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

Junio C Hamano <gitster@pobox.com> writes:

> My gut feeling, if I have to choose between "lazy loading" and
> "popluate in prepare_repo_settings() and then access the member
> directly thereafter" for this variable, I may pick the latter for
> this particular variable.

I left the reason behind this choice unsaid, but let's spell it out.

Originally, this was read in git_config(git_default_config) into a
global, and that is probably because almost everybody that touches
the working tree files needs to know about it.  So populating it in
prepare_repo_settings() for everybody, even though the calling code
path does not even need it, would be OK---they were paying the cost
to read it when they read the default configuration variables.

It seems Patrick earlier made a confused comment on the two models
that may need a bit clarifying.

Here are the rules to follow.

 - "lazy loading" is not wrong. Initialize the member to an
   "uninitialized" state, never touch the member in
   prepare_repo_settings(), and have its getter check for the
   "uninitialized" state to lazily load it, or have its setter do
   its thing.  prepare_repo_settings() should not even be aware of
   the member, if we are going to give the member a getter/setter
   pair.

 - "Without getter/setter" is not wrong, either.  Load the member in
   prepare_repo_settings(), which will turn into a no-op once it is
   called to a repo instance.  Use the member directly afterwards.

You cannot mix and match.  If the variable is rarely used, you'd
want to catch the initial access and lazily load it, hence you are
required to have a getter/setter pair and lazily populate the member
in your getter.  If your variable is very commonly used, load it
once in prepare_repo_settings(), and because you are not going to do
anything special upon the first access, there is no need to have a
getter/setter pair.