Michael Haggerty <mhagger@xxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html