Hi
On 16/02/17 23:56, Jeff Gilbert wrote:
I don't visually like ||/&& at start of line, but I can't fault the reasoning, so I'm weakly for it. I don't think it's important enough to change existing code though.
Disclaimer: I'm now biased about this.I've been writing C and C++ code for now most of my life. And during that time you take strong habits. Not always good habits, but there are there.
Now having operators at the end of the line, including logical ones like && and || is something I've always done, because it's just the way things were done. Never really thought much about it, you place the operator at the end of the line when you're splitting a long logical operations.
but is it the proper things to do?Just like Gerald above, I had misread (or more accurately understood differently) what the rules were in that document, because there's various discrepancies in it.
When you read code, especially during review, the review process is made much more easily when you can at a glance understand one particular logical calculation.
reading this: (turn on courier)
return ((aCodecMask & VPXDecoder::VP8)
&& aMimeType.EqualsLiteral("video/webm; codecs=vp8"))
|| ((aCodecMask & VPXDecoder::VP9)
&& aMimeType.EqualsLiteral("video/webm; codecs=vp9"))
|| ((aCodecMask & VPXDecoder::VP9)
&& aMimeType.EqualsLiteral("video/vp9"));
than:
return ((aCodecMask & VPXDecoder::VP8) &&
aMimeType.EqualsLiteral("video/webm; codecs=vp8")) ||
((aCodecMask & VPXDecoder::VP9) &&
aMimeType.EqualsLiteral("video/webm; codecs=vp9")) ||
((aCodecMask & VPXDecoder::VP9) &&
aMimeType.EqualsLiteral("video/vp9"));
where does the || apply, where does the && ?
I must recompose the entire expression in my mind to understand what's
going on.
The previous one require no such mental process.If we are talking about new code, the question becomes, can this become the rule *and* can we add such rule to clang-format seeing that we're in the process of applying clang-format to the entire source tree (with exceptions), modifying existing code would become a moot issue.
Cheers JY
On Thu, Feb 16, 2017 at 1:47 PM, <gsquel...@mozilla.com> wrote:Question of the day: When breaking overlong expressions, should &&/|| go at the end or the beginning of the line? TL;DR: Coding style says 'end', I&others think we should change it to 'beginning' for better clarity, and consistency with other operators. Our coding style reads: "Break long conditions after && and || logical connectives. See below for the rule for other operators." [1] """ Overlong expressions not joined by && and || should break so the operator starts on the second line and starts in the same column as the start of the expression in the first line. This applies to ?:, binary arithmetic operators including +, and member-of operators (in particular the . operator in JavaScript, see the Rationale). Rationale: operator at the front of the continuation line makes for faster visual scanning, because there is no need to read to end of line. Also there exists a context-sensitive keyword hazard in JavaScript; see bug 442099, comment 19, which can be avoided by putting . at the start of a continuation line in long member expression. """ [2] I initially focused on the rationale, so I thought *all* operators should go at the front of the line. But it seems I've been living a lie! &&/|| should apparently be at the end, while other operators (in some situations) should be at the beginning. Now I personally think this just doesn't make sense: - Why the distinction between &&/|| and other operators? - Why would the excellent rationale not apply to &&/||? - Pedantically, the style talks about 'expression *not* joined by &&/||, but what about expression that *are* joined by &&/||? (Undefined Behavior!) Based on that, I believe &&/|| should be made consistent with *all* operators, and go at the beginning of lines, aligned with the first operand above. And therefore I would propose the following changes to the coding style: - Remove the lonely &&/|| sentence at [1]. - Rephrase the first sentence at [2] to something like: "Overlong expressions should break so that the operator starts on the following line, in the same column as the first operand for that operator. This applies to all binary operators, including member-of operators (in particular the . operator in JavaScript, see the Rationale), and extends to ?: where the 2nd and third operands should be on separate lines and start in the same column as the first operand." - Keep the rationale at [2]. Also, I think we should add something about where to break expressions with operators of differing precedences, something like: "Overlong expressions containing operators of differing precedences should first be broken at the operator of lowest precedence. E.g.: 'a+b*c' should be split at '+' before '*'" A bit more context: Looking at the history of the coding style page, a certain "Brendan" wrote that section in August 2009 [3], shortly after a discussion here [4] that seemed to focus on the dot operator in Javascript. In that discussion, &&/|| appear in examples at the end of lines and nobody talks about that (because it was not the main subject, and/or everybody agreed with it?) Discuss! [1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Control_Structures [2] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators [3] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style$compare?locale=en-US&to=7315&from=7314 [4] https://groups.google.com/d/msg/mozilla.dev.platform/Ji9lxlLCYME/zabUmQI9S-sJ _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform_______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
