Olga Telezhnaya <olyatelezhnaya@xxxxxxxxx> writes:
> Get rid of goto command in ref-filter for better readability.
>
> Signed-off-by: Olga Telezhnaia <olyatelezhnaya@xxxxxxxxx>
> Mentored-by: Christian Couder <christian.couder@xxxxxxxxx>
> Mentored by: Jeff King <peff@xxxxxxxx>
> ---
How was this patch "mentored by" these two folks? Have they already
reviewed and gave you OK, or are you asking them to also help reviewing
with this message? Mostly just being curious.
It is not convincning that this splitting the last part of a single
function into a separate helper function that is called from only
one place improves readability. If better readability is the
purpose, I would even say
for (i = 0; i < used_atom_cnt; i++) {
if (...)
- goto need_obj;
+ break;
}
- return;
+ if (used_atom_cnt <= i)
return;
-need_obj:
would make the result easier to follow with a much less impact.
If we later in the series will use this new helper function from
other places, it certainly makes sense to create a new helper like
this patch does, but then "get rid of goto for readability" is not
the justification for such a change.
> ref-filter.c | 103 ++++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 53 insertions(+), 50 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index f9e25aea7a97e..37337b57aacf4 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1354,16 +1354,60 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
> return show_ref(&atom->u.refname, ref->refname);
> }
>
> +static void need_object(struct ref_array_item *ref) {
Style. The opening brace at the beginning of the function sits on
its own line alone.
