<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 16, 2017 at 2:54 PM, Matthew Jordan <span dir="ltr"><<a href="mailto:mjordan@digium.com" target="_blank">mjordan@digium.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Warning: bike shedding.<br>
<br>
On several code reviews, comments have been left along with a -1 on<br>
the review - indicating that it is not acceptable for merging - due to<br>
line length restrictions. While this is technically correct per our<br>
coding guidelines, it doesn't always result in the most readable code.<br>
<br>
Personally, I find this:<br>
<br>
if (a_good_variable_name-><wbr>another_good_name &&<br>
do_something_to_it(a_good_<wbr>variable_name->another_good_<wbr>name)) {<br>
// Do things<br>
}<br>
<br>
To be preferable to:<br>
<br>
if (a_good_variable_name-><wbr>another_good_name<br>
&& do_something_to_it(a_good_<wbr>variable_name->another_good_<wbr>name)) {<br>
// Do things<br>
}<br>
<br>
This is typically because the indentation of the second part of the<br>
boolean test lines up with the code that is to be executed if both<br>
conditions is true, which is more visually jarring than just reading<br>
the first line.<br>
<br>
I'll grant this is probably a personal preference, but we have<br>
certainly opted for the former over the latter in many places in the<br>
code.<br></blockquote><div><br></div><div>It is done both ways in the code. <br></div><div><br></div><div>My preference is the opposite for a couple reasons:<br></div><div>1) Seeing && at the beginning of a line you then know that it is part of a line<br>continuation because && is a binary operator and needs something to its<br></div><div>left.<br></div><div>2) On long lines you can miss seeing that there even is an operator on the<br>end of the line. The line end may even be off the edge of the window. Either<br></div><div>way you would not know how to interpret the expression unless you knew<br>what the operator is. You are then forced to scroll the window to find out.<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
While I know that we aren't all sitting around with dual screens, we<br>
also aren't living in 1979 with dumb terminals. The line length<br>
restriction of 90 columns seems reasonable until you end up with long<br>
function names, which - since we live in a land of void * - is<br>
generally something that we should encourage. I'd like to say that<br>
some common sense about when to apply the line length rule and when<br>
not to would "work for us", but that doesn't appear to be the case.<br>
<br>
Questions than:<br>
(1) Should there even be a line length rule?<br></blockquote><div><br></div><div>Yes. We live in the age of variable sized and multi-overlapping windows.<br></div><div>In a way this is worse than the 1979's fixed sized display because we<br></div><div>try to cram more on the screen.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
(2) If there is a line length, what is a reasonable length given some<br>
of our function names? (Looking at things like<br>
ast_sip_get_mwi_disable_<wbr>initial_unsolicited)<br></blockquote><div><br></div><div>The 90 line length is a good compromise. You can look at side by side<br></div><div>differences and not have to scroll horizontally except when the line length<br></div><div>is too long.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
(3) Should we simply advocate for readability, with examples of what<br>
is readable and what is not?<br></blockquote><div><br></div><div>There are ways to deal with the long identifier lengths creating very long<br>lines. One of the simpler is to avoid putting assignments in conditional<br></div><div>test expressions. There are better readability line breaks available in a<br></div><div>stand alone assignment than there are when you cram the assignment<br></div><div>into an if test expression. Another is not to excessively indent parameters<br></div><div>to functions like python PEP8 encourages. Function parameters tend<br>to get quite long themselves. Horizontal line space is at a premium<br>compared to vertical space.<br></div><div><br></div><div>Richard<br></div><br></div></div></div>