Remove const declaration of POD locals in actions bar code

Remove const declaration of POD locals in actions bar code

As a follow up to the discussion in
https://codereview.chromium.org/1469423002/, remove
the const declaration of POD local variables in
the actions bar code.

BUG=555031, 550816
TEST=none

[tdanderson, 2015-12-09 19:57:10]

tdanderson@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[tdanderson, 2015-12-09 19:57:10]

Hi Devlin, please take a look.

[Devlin, 2015-12-09 20:01:45]

lgtm.

Thanks :)

[tdanderson, 2015-12-09 20:08:05]

On 2015/12/09 20:01:45, Devlin wrote:
> lgtm.
> 
> Thanks :)

np :)

[tdanderson, 2015-12-09 20:08:12]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2015-12-09 20:11:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1505403003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1505403003/1

[Peter Kasting, 2015-12-09 21:01:39]

The CQ bit was unchecked by pkasting@chromium.org

[Peter Kasting, 2015-12-09 21:06:53]

Not LGTM.

I own the c/b/ui code and encourage local consts as much as possible.  Terry's
original change is in keeping with that pattern.

The Google style guide also now encourages developers to use const wherever
possible and has removed older language that sounded too discouraging about
const.  I think using local consts is more consistent with the style guide.  The
comments about this on the previous CL mention that the style guide encourages
const in "three main cases", but I think that's a misrepresentation of the
guide.  The three cases listed are illustrative examples of why const is a win
in some different scenarios, but the intent of the style guide is to encourage
const everywhere, which is why the initial sentence is intentionally worded as
broadly as possible.

So I would like this left as-is.

[Devlin, 2015-12-09 21:23:46]

On 2015/12/09 21:06:53, Peter Kasting wrote:
> Not LGTM.
> 
> I own the c/b/ui code and encourage local consts as much as possible.  Terry's
> original change is in keeping with that pattern.
> 
> The Google style guide also now encourages developers to use const wherever
> possible and has removed older language that sounded too discouraging about
> const.  I think using local consts is more consistent with the style guide. 
The
> comments about this on the previous CL mention that the style guide encourages
> const in "three main cases", but I think that's a misrepresentation of the
> guide.  The three cases listed are illustrative examples of why const is a win
> in some different scenarios, but the intent of the style guide is to encourage
> const everywhere, which is why the initial sentence is intentionally worded as
> broadly as possible.
> 
> So I would like this left as-is.

I read "Use const whenever it makes sense" from the style guide. [1]  My whole
point here was that, IMO, local consts on POD don't really make sense (on
objects, local consts are great).  Is there an updated style guide encouraging
broader use?

https://google.github.io/styleguide/cppguide.html#Use_of_const

[Peter Kasting, 2015-12-09 21:33:29]

On 2015/12/09 21:23:46, Devlin wrote:
> On 2015/12/09 21:06:53, Peter Kasting wrote:
> > Not LGTM.
> > 
> > I own the c/b/ui code and encourage local consts as much as possible. 
Terry's
> > original change is in keeping with that pattern.
> > 
> > The Google style guide also now encourages developers to use const wherever
> > possible and has removed older language that sounded too discouraging about
> > const.  I think using local consts is more consistent with the style guide. 
> The
> > comments about this on the previous CL mention that the style guide
encourages
> > const in "three main cases", but I think that's a misrepresentation of the
> > guide.  The three cases listed are illustrative examples of why const is a
win
> > in some different scenarios, but the intent of the style guide is to
encourage
> > const everywhere, which is why the initial sentence is intentionally worded
as
> > broadly as possible.
> > 
> > So I would like this left as-is.
> 
> I read "Use const whenever it makes sense" from the style guide. [1]  My whole
> point here was that, IMO, local consts on POD don't really make sense (on
> objects, local consts are great).

I read your comments, but I was not convinced that local consts on POD types
don't make sense.  The readability benefit of knowing the value doesn't change
is irrespective of whether the local is of POD type.

>  Is there an updated style guide encouraging
> broader use?

This is the updated style guide.  The old guide talked a bit about not using
consts "excessively" and that was removed after it turned out to discourage
people from using const a bit more than was intended.

[Devlin, 2015-12-09 21:56:28]

On 2015/12/09 21:33:29, Peter Kasting wrote:
> I read your comments, but I was not convinced that local consts on POD types
> don't make sense.  The readability benefit of knowing the value doesn't change
> is irrespective of whether the local is of POD type.

The difference for POD vs. non-POD is that with a POD type, it should almost
always be immediately clear if the value changes by looking at the code in the
function, because the operations that can change a POD value are pretty limited
and, hopefully, self-evident.  With a non-POD type, that's not the case - if I
call a method, I have to look at that class's definition in order to determine
whether or not it will modify the object.

It seems like putting const on function-local POD types is just added noise.  If
I care whether or not it's const, it should be clear from the surrounding code. 
Adding const makes it (very slightly) more work on me as the reader and (again
very slightly) more work on whoever later needs to modify the variable - all
with no benefit over just looking at where the variable is used (which, if you
care about const-ness, presumably you are already looking at).

[Peter Kasting, 2015-12-09 22:01:35]

On 2015/12/09 21:56:28, Devlin wrote:
> On 2015/12/09 21:33:29, Peter Kasting wrote:
> > I read your comments, but I was not convinced that local consts on POD types
> > don't make sense.  The readability benefit of knowing the value doesn't
change
> > is irrespective of whether the local is of POD type.
> 
> The difference for POD vs. non-POD is that with a POD type, it should almost
> always be immediately clear if the value changes by looking at the code in the
> function, because the operations that can change a POD value are pretty
limited
> and, hopefully, self-evident.

That requires you to scan all the code in a function to know what happens. 
"const" tells you immediately, right where you're looking.

The major work here is in scanning the function.  The issue of having to know
what class methods do is comparatively minor; generally you know what the
methods do or can make reasonable guesses.

> It seems like putting const on function-local POD types is just added noise. 
If
> I care whether or not it's const, it should be clear from the surrounding
code.

Again, the goal is to never look at any surrounding code, which is why this is
_less_ work for a reader, not more.  There's simply no way to argue that even
the most rapid scan of a function is "less work" than reading a single word at a
declaration site.
 
> Adding const makes it (very slightly) more work on me as the reader and (again
> very slightly) more work on whoever later needs to modify the variable

Local const has caught bugs in my own changes where I accidentally modified
values I shouldn't have.  It's also frequently helped prod me into writing
better changes -- often a variable being const makes me think about whether
making it non-const is really the best way of doing something, and I end up
finding a clearer method.

So, for the benefit of both reading and writing, my experience matches the style
guide recommendations: use const wherever possible.

[tdanderson, 2015-12-17 21:10:37]

On 2015/12/09 22:01:35, Peter Kasting wrote:
> On 2015/12/09 21:56:28, Devlin wrote:
> > On 2015/12/09 21:33:29, Peter Kasting wrote:
> > > I read your comments, but I was not convinced that local consts on POD
types
> > > don't make sense.  The readability benefit of knowing the value doesn't
> change
> > > is irrespective of whether the local is of POD type.
> > 
> > The difference for POD vs. non-POD is that with a POD type, it should almost
> > always be immediately clear if the value changes by looking at the code in
the
> > function, because the operations that can change a POD value are pretty
> limited
> > and, hopefully, self-evident.
> 
> That requires you to scan all the code in a function to know what happens. 
> "const" tells you immediately, right where you're looking.
> 
> The major work here is in scanning the function.  The issue of having to know
> what class methods do is comparatively minor; generally you know what the
> methods do or can make reasonable guesses.
> 
> > It seems like putting const on function-local POD types is just added noise.

> If
> > I care whether or not it's const, it should be clear from the surrounding
> code.
> 
> Again, the goal is to never look at any surrounding code, which is why this is
> _less_ work for a reader, not more.  There's simply no way to argue that even
> the most rapid scan of a function is "less work" than reading a single word at
a
> declaration site.
>  
> > Adding const makes it (very slightly) more work on me as the reader and
(again
> > very slightly) more work on whoever later needs to modify the variable
> 
> Local const has caught bugs in my own changes where I accidentally modified
> values I shouldn't have.  It's also frequently helped prod me into writing
> better changes -- often a variable being const makes me think about whether
> making it non-const is really the best way of doing something, and I end up
> finding a clearer method.
> 
> So, for the benefit of both reading and writing, my experience matches the
style
> guide recommendations: use const wherever possible.

Closing this out.