Re: [PATCH 1/9] tmp_objdir: add a helper function for discarding all contained objects
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 22, 2021, 06:23 UTC
- Message-ID
- <xmqqlf0dkvig.fsf@gitster.g>
- In-Reply-To
- <CABPp-BEuzorQkesgyNCvS05wsfqaH4Yv5JDFgy+AenrAC+JJNQ@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
Show 19 quoted lines
>> > +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 <newren@gmail.com> >> > >> > Signed-off-by: Elijah Newren <newren@gmail.com>
Thanks.