# Re: \[PATCH 2/5\] log: Refactor duplicated code to headerize recipient lists

- From: Junio C Hamano \<hidden\>
- Date: 2023-01-25 02:11:22 
- List: git
- Page: https://ratatoskr.run/git/2023/01/7964044
- Thread: https://ratatoskr.run/git/2023/01/7964044/t.md

```text
Zev Weiss [ref] writes:

> Subject: Re: [PATCH 2/5] log: Refactor duplicated code to headerize recipient lists

Style: "log: Refactor" -> "log: refactor"

cf. Documentation/SubmittingPatches[[summary-section]]

> The To and Cc headers are handled identically (the only difference is
> the header name tag), so we might as well reuse the same code for both
> instead of duplicating it.

Makes tons of sense.  But seeing this one ...

> +	recipients_to_header_buf("To", &buf, &extra_to);
> +	recipients_to_header_buf("Cc", &buf, &extra_cc);

... the parameters to the function probably should be ...

> +void recipients_to_header_buf(const char *hdr, struct strbuf *buf,
> +			      const struct string_list *recipients);

... in this order, instead:

    format_recipients(&buf, "To", &extra_to);

That is, "To" and &extra_to are much closely related to each other
than they are to &buf in the sense that they are both input to the
helper function to work on, while &buf is an output buffer, and we
tend to place closer things together, next to each other.

Other than that, removal of repetition is very good.

Thanks.
```