Re: [PATCH v3 8/8] builtin/checkout-index: stop using `the_repository`

[Usman Akinyemi <usmanakinyemi202@gmail.com>]

On Fri, Mar 7, 2025 at 7:45 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Usman Akinyemi <usmanakinyemi202@gmail.com> writes:
>
> >> Hmph, if we are passing anything down to these code paths, I would
> >> have expected that it would be an instance of "struct index_state".
> >>
> >> Do these two helper functions need anything other than that from the
> >> repository instance?
> > No, they do not. They could possibly do in the future and is there any
> > reason why we might want to pass the "struct index_state" instead of
> > the whole "struct repository" ?
>
> You are asking a wrong question.
>
> When there are two things, A and B, where B is a piece of data with
> a smaller scope that is contained within A, unless your function
> needs to access parts of A that is impossible to access if you fed
> it only B, limiting the interface to the smallest piece necessary
> (in this case, B) is always the better design.  A potential caller
> that only has B and not A can still call your function that takes B
> if you designed the API that way.  If you insist that the caller
> pass A (and infer which B to use as part of A), a potential caller
> that only has B cannot call your function.
>
> So, with that understanding of a general principle in mind, you need
> a stronger justification to pass the larger object A that goes
> against the best practice, saying "Even though we do not have to
> pass A because only B is necessary, we pass A because ...".  And you
> just said there is no such reason why you need a repository and for
> these functions an index_state is sufficient.
>
> There are many functions that only need "struct index_state"
> instance and obtains one from their callers.  They do not have a
> repository object, other than the one that is referred to from the
> index_state (to keep track of which repository it originated from).
>
> But this pointer is not designed to round-trip.  There can be, and
> there indeed are, code paths that use multiple index_state instances
> associated with one repository.  But a repository instance has only
> one ".index" member.  It means if you pass a repository, you are
> making it impossible for your potential callers that has secondary
> index_state instances.
Thanks for the explanation.

I did not know or think about this before.

I will make changes to this in the next iteration.

Thank you.
>