On Jan 2, 2014, at 1:40 PM, Dan Bernstein <m...@apple.com> wrote:
>
> On Jan 2, 2014, at 1:12 PM, Geoffrey Garen <gga...@apple.com> wrote:
>
>> Hi folks.
>>
>> Recently, I’ve seen patches go by using the C++11 “auto” keyword. For
>> example, let me pick on Andreas:
>>
>>> <http://trac.webkit.org/changeset/161143>
>>>
>>> + auto newRenderer = textNode.createTextRenderer(style);
>>> + ASSERT(newRenderer);
>>>
>>> ….
>>>
>>> + parentRenderer->addChild(newRenderer.leakPtr(), nextRenderer);
>>
>> I think this use of “auto” is net harmful.
>>
>> Upsides:
>>
>> - Less typing
>>
>> Downsides:
>>
>> - I don’t know the type of ’newRenderer’ at a glance
>> - 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).
>>
>> 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.
>
> We tend to avoid local write-once-read-once local variables, i.e. we tend to
> prefer
>
> {
> setSize(optimalSize());
> }
>
> to
>
> {
> CGSize newSize = optimalSize();
> setSize(newSize);
> }
>
> 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?
This is an interesting analogy. In the past, I have used write-once-read-once
locals in order to 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.
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:
auto newSize = optimalSize();
or this:
CGSize newSize = optimalSize();
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.
On the other hand, I do agree that:
setSize(optimalSize())
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().
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.
-Filip
> _______________________________________________
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
