Junio C Hamano <gitster@xxxxxxxxx> writes: >> However, I believe that some questions *shouldn't* be answered by >> just struct branch. The prime example in this series is >> branch_get_push() - it conceptually answers 'What is the pushremote >> of this branch', but the behavior we want is closer to 'If >> configured, give me the pushremote for the branch. Otherwise, give me >> the default pushremote of the repo.'. This is arguably a relevant >> detail that should be exposed to callers. > > It is a good example why such a function can just take an instance > of branch, and the caller, > > (1) who does care about the fallback, can be assured that the logic > falls back to the correct repository; and > > (2) who does not care about the fallback and sees it a mere > implementation detail of "I am on this branch; give me the > remote to push to", do not have to know what, other than the > branch object, needs to be passed. > > if we explicitly record a branch object which repository it was > taken from. > > There may be some other (real) reason where the resistance comes > from, that you may not be telling us, though. But in what was > described in the message I am responding to, I didn't see much > convincing reason to argue _for_ keeping the contained objects > ignorant of the container and forcing callers to pass both to > functions that use both the container and contained to compute > something. My primary line of thinking is that adding the backpointer from struct branch to struct repository adds "unnecessary" coupling between the two objects, which causes the more concrete problems of: * Inconsistency between other functions that use struct repository as a "context object". We now find ourselves having to justify why branch functions can bypass the context object using a special pointer, whereas other functions and structs (say, struct object) cannot. * Interface misuse, where callers can now dereference branch->repository even though it is meant to be internal. This is also a possible source of inconsistency. * Extra memory consumption. A counterpoint to what I said is [1], where Jonathan eventually added the backpointer from struct ref_store to struct repository, which does give the nice benefits of referential integrity and abstraction that you cited. However in most of that series, struct ref_store is meant to be the context object, but due to poor internal abstraction, ref_store ends up depending on repository in some form or another and thus the backpointer is more defensible (as opposed to passing two context objects). I do not think we are in the same position with struct branch; struct branch seems sufficiently separated to me. I'm not opposed to adding a backpointer if it helps us get to a good final state, but I currently suspect that the final state still involves passing around struct repository as a context object. [1] https://lore.kernel.org/git/xmqqlf3gib0p.fsf@gitster.g/
