<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Jan 2, 2014, at 1:40 PM, Dan Bernstein <<a href="mailto:mitz@apple.com">mitz@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>On Jan 2, 2014, at 1:12 PM, Geoffrey Garen <<a href="mailto:ggaren@apple.com">ggaren@apple.com</a>> wrote:<br><br><blockquote type="cite">Hi folks.<br><br>Recently, I’ve seen patches go by using the C++11 “auto” keyword. For example, let me pick on Andreas:<br><br><blockquote type="cite"><<a href="http://trac.webkit.org/changeset/161143">http://trac.webkit.org/changeset/161143</a>><br><br>+ auto newRenderer = textNode.createTextRenderer(style);<br>+ ASSERT(newRenderer);<br><br>….<br><br>+ parentRenderer->addChild(newRenderer.leakPtr(), nextRenderer);<br></blockquote><br>I think this use of “auto” is net harmful.<span class="Apple-converted-space"> </span><br><br>Upsides:<br><br>- Less typing<br><br>Downsides:<br><br>- I don’t know the type of ’newRenderer’ at a glance<br>- Navigating to newRenderer’s class definition is a two-step process: (1) Go to the definition of createTextRenderer and see what it returns; (2) Go to the definition of (1).<br><br>I think the downsides outweigh the upsides here because reading code is more common than writing code, and because it’s not *that* much typing to write out "RenderPtr< RenderText>”. In this particular code, I think it’s especially bad to disguise the type of a pointer in the render tree, since we’re in the middle of a transition to a new type of pointer, and so it’s important to know if you’re looking at code that does the new thing or the old thing.<br></blockquote><br>We tend to avoid local write-once-read-once local variables, i.e. we tend to prefer<br><br>{<br> setSize(optimalSize());<br>}<br><br>to<br><br>{<br> CGSize newSize = optimalSize();<br> setSize(newSize);<br>}<br><br>But the up- and down-sides of this are the same as those of using auto. Do you think we should also encourage use of write-once-read-once locals for the same reasons?<br></div></blockquote><div dir="auto"><br></div><div dir="auto">This is an interesting analogy. In the past, I have used write-once-read-once locals <i>in order to</i> explicitly expose the variable's type. On the other hand, omitting the temporary variable can increase readability by removing noise. Probably, it's usually better to omit write-once-read-once variables, and so that should probably be a style suggestion. I don't think we should change this.</div><div dir="auto"><br></div><div dir="auto">I think the goal should be that if you do introduce a temporary variable for some reason, then being explicit about its type is better. Consider that in your second example, there might be 200 lines of code between the call to optimalSize() and the call to setSize(). I that case, we're essentially choosing between having the code reader see this:</div><div dir="auto"><br></div><div dir="auto"> auto newSize = optimalSize();</div><div dir="auto"><br></div><div dir="auto">or this:</div><div dir="auto"><br></div><div dir="auto"> CGSize newSize = optimalSize();</div><div dir="auto"><br></div><div dir="auto">If I'm trying to grok this code, I will be looking for any hint I can find to determine what the heck newSize is. In the first version, I just know that it's called "newSize" and that it's the result of calling a function called "optimalSize()". In the second version, I know all of those facts, plus I know that it's a CGSize. That's one more bit of information that I can use to build up my intuition about the code. And in this case, I get that extra information with just two more characters of typing.</div><div dir="auto"><br></div><div dir="auto">On the other hand, I do agree that:</div><div dir="auto"><br></div><div dir="auto"> setSize(optimalSize())</div><div dir="auto"><br></div><div dir="auto">is the best: in this case I'm not going to even think about any temporary variables because I already know what the code is doing just by looking at it. But it's not always possible to write the code that way even if the variable is write-once-read-once. For example, in the 200 lines of code between the call to optimalSize() and the call to setSize(), I might clobber the state necessary to compute optimalSize().</div><div dir="auto"><br></div><div dir="auto">So, to me, the policy should be: *if* you already have to have a variable for something *then* you should prefer to be explicit about its type.</div><div dir="auto"><br></div><div dir="auto">-Filip</div><div dir="auto"><br></div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">_______________________________________________<br>webkit-dev mailing list<br><a href="mailto:webkit-dev@lists.webkit.org">webkit-dev@lists.webkit.org</a><br><a href="https://lists.webkit.org/mailman/listinfo/webkit-dev">https://lists.webkit.org/mailman/listinfo/webkit-dev</a></div></blockquote></div><br></body></html>