From: Junio C Hamano Date: Wed, 22 Dec 2021 06:23:51 GMT Subject: Re: [PATCH 1/9] tmp_objdir: add a helper function for discarding all contained objects Message-ID: In-Reply-To: Elijah Newren writes: >> > +void tmp_objdir_discard_objects(struct tmp_objdir *t) >> > +{ >> > + remove_dir_recursively(&t->path, REMOVE_DIR_KEEP_TOPLEVEL); >> > +} >> > + >> >> OK. >> >> Without a caller, it is a bit hard to judge if a separate helper >> makes the caller easier to read and understand, or becomes an extra >> layer of abstraction that obscures the logic. Hopefully, having a >> more specific function name with "tmp" and "discard" in it makes the >> intent at callers more clear than the function that is named after >> the detail of the operation. > > This isn't just a convenience; since tmp_objdir is defined in > tmp-objdir.c rather than tmp-objdir.h, t->path is not accessible > outside of tmp-objdir.c. Because of this, some kind of helper > function is necessary. But adding this function as an extra level of abstration is *not* the only way to expose the feature. Instead the internal of "struct tmp_objdir" could be exposed to the caller that wants to discard the files inside the path. I think we now have enough material to fill between these two lines to help readers ;-) >> > From: Elijah Newren >> > >> > Signed-off-by: Elijah Newren Thanks.