On Fri, Mar 7, 2025 at 7:45 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Usman Akinyemi <usmanakinyemi202@xxxxxxxxx> 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. >
