Re: [PATCH 1/2] gpg-interface: refactor 'enum sign_mode' parsing

>
> Patrick Steinhardt <ps@pks.im> writes:
>
> >> +    if (!parse_sign_mode(arg, val))
> >> +            return 0;
> >> +
> >> +    return error("Unknown %s mode: %s", opt->long_name, arg);
> >
> > Would it make sense to maybe reverse the error handling and say
> > something like:
> >
> >     if (parse_sign_mode(arg, val) < 0)
> >         return error("Unknown %s mode: %s", opt->long_name, arg);
> >
> >     return 0;
> >
> > That reads a bit more natural to me at least.

> I also thought that the original had a "Huh?" factor, but my
> reaction was more like "wouldn't it be easier to see if these are
> both sides of if/else?", i.e.
>
>         if (parse_sign_mode(...))
>                 return error(...);
>         else
>                 return 0;
>
> which made me think that the differences were mostly subjective and
> refrained from commenting.

> But after I see your version, I tend to agree that it is easier to
> see the flow if the most-straightforward success case ran through to
> the end, while exceptional cases (i.e. an error return, or the
> initial "unset" case) did early returns.  I have to add, however,
> that I feel that this is mostly subjective and falls into "once it
> is in the tree, it's not really worth the patch noise to go and fix
> it up" category.  If we will have v2 of this series, I do prefer to
> see it written more in your way, though.