On Thu, Mar 13, 2014 at 2:34 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Eric Sunshine <sunsh...@sunshineco.com> writes:
>> Shouldn't this logic [to decide what the printf arguments should
>> be] also be encoded in the table?
>> ...
>> The same argument also applies to computation of the 'name' variable
>> above. It too can be pushed into the the table.
> Because the "printf argument" logic does not have to be in the
> table, the same argument does not apply to the 'name' thing.
> After looking at the v5 patch, I do not think an extra two-element
> array to switch between remote vs shortname is making it any easier
> to read.  I would have to say that personally I find that
>         const char *name[] = {remote, shortname};
>         ... long swath of code ...
>         printf_ln(... name[!remote_is_branch] ...);
> is a lot harder to read than:
>         printf_ln(... remote_is_branch ? shortname : branch ...);

Indeed, that's a step backward, and is not what was asked. Merely
pushing data into tables does not make the logic table-driven
(emphasis on *driven*). The GSoC microproject did not demand a
table-driven approach, but instead asked students if such an approach
would make sense. A more table-driven approach might look something
like this:

    struct M { const char *s; const char **a1; const char **a2; }
    message[][2][2] = {{{
        { "Branch %s set ... %s ... %s", &shortname, &origin },
        { "Branch %s set ... %s", &remote, NULL },

    const struct M *m = message[!remote_is_branch][!origin][!rebasing];
    printf_ln(m->s, local, *m->a1, *m->a2);

Whether this approach is more clear than the original code is a matter
for debate [1], however, it's obvious from the table which arguments
belong with each message, and the printf_ln() invocation does not
require any logic. When moving only messages into a table, they become
disconnected from their arguments which makes reasoning about them a
bit more difficult. The original code does not have this problem, nor
does a table-driven approach.

[1]: While ungainly, the original code may not be sufficiently bad to
warrant the extra complications of a table. A simple refactoring, such
as [2], can make the code a bit easier to read without adding

[2]: http://thread.gmane.org/gmane.comp.version-control.git/243704
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to