Michael Haggerty <mhag...@alum.mit.edu> writes:
> In projects where I can choose the coding standard, I like to use extra
> whitespace to help the eye pick out the range of parentheses, like
>
> if (!(
> (flags & DO_FOR_EACH_INCLUDE_BROKEN) ||
> ref_resolves_to_object(entry)
> ))
> return 0;
>
> but I've never seen this style in the Git project so I haven't used it here.
I _think_ at least to me, it is not the textual grouping style but
the "unless X we skip the rest" logic itself that confused me. It
took the same number of minutes to grok the above as the original.
We skip the rest of this function unless the condition inside !()
holds, and the condition is.... we are told to include broken ones,
in which case we know we will do the remainder without checking
anything else, or we do the checking and find that it is not broken.
I suspect the root cause of the confusion to force the double
negation is INCLUDE_BROKEN; we have to express "when we are told to
ignore broken one and this thing is broken, ignore it" without
negation, i.e.
if ( !(flags & INCLUDE_BROKEN) && is_broken(thing) )
return;
which would have been much more pleasant to read if it were
if ( (flags & IGNORE_BROKEN) && is_broken(thing) )
return;
Unfortunately, we cannot change it to IGNORE_BROKEN and flip the
meaning of the bit, because almost all callers do want to ignore
broken ones.
Either way is fine by me, even though I find that !(A || B) needs a
bit more mental exercise to grok than (!A && !B). Perhaps it is
just me who is not so strong at math ;-)
--
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
