Eric Sunshine <sunsh...@sunshineco.com> writes:
> Junio labeled the "-2" trick "cute", and while it is optimal in that
> it only traverses the key/value list once, it also increases cognitive
> load since the reader has to spend a good deal more brain cycles
> understanding what is going on than would be the case with simpler
> (and less noisily repetitive) code.
... and update three copies of very similar looking code that have
to stay similar. If this were parsing unbounded and customizable
set of variables, I think the approach you suggest to use a helper
that iterates over the whole array for each key makes sense, but for
now I think what was queued would be OK.
> An alternative would be to make the code trivially easy to understand,
> though a bit more costly, but that expense should be negligible since
> this file should always be tiny, containing very few key/value pairs,
> and it's not timing critical code anyhow. For instance:
>
> static char *find(struct string_list *kv, const char *key)
> {
> const char *val = NULL;
> int i;
> for (i = 0; i < kv.nr; i++) {
> if (!strcmp(kv.items[i].string, key)) {
> if (val) {
> error(_("duplicate %s"), key);
> return NULL;
> }
> val = kv.items[i].util;
> }
> }
> if (!val)
> error(_("missing %s"), key);
> return val;
> }
>
> static int read_author_script(struct am_state *state)
> {
> ...
> char *name, *email, *date;
> ...
> name = find(&kv, "GIT_AUTHOR_NAME");
> email = find(&kv, "GIT_AUTHOR_EMAIL");
> date = find(&kv, "GIT_AUTHOR_DATE");
> if (name && email && date) {
> state->author_name = name;
> state->author_email = email;
> state->author_date = date;
> retval = 0;
> }
> string_list_clear&kv, !!retval);
> ...
