[tor-bugs] #26184 [Applications/Tor Browser]: Think about using `const` as much as possible in Torbutton code

[Tor Bug Tracker & Wiki]

#26184: Think about using `const` as much as possible in Torbutton code
--------------------------------------+--------------------------
 Reporter:  gk                        |          Owner:  tbb-team
     Type:  task                      |         Status:  new
 Priority:  Medium                    |      Milestone:
Component:  Applications/Tor Browser  |        Version:
 Severity:  Normal                    |     Resolution:
 Keywords:  tbb-torbutton             |  Actual Points:
Parent ID:                            |         Points:
 Reviewer:                            |        Sponsor:
--------------------------------------+--------------------------

Comment (by acat):

 > Is Mozilla doing this?

 I think most of the Firefox JS code uses `let` for declarations, but I did
 not see any general guidelines for this. Some components seem to be
 enforcing `const` though, like `devtools`:
 https://bugzilla.mozilla.org/show_bug.cgi?id=1454696.

 I think using `const` as  much as possible does add value if it's clear
 what it means, like pospeselr said: `when the reference is used it is
 pointing to the same object throughout the block`. It does not mean that
 the object of a `const` declaration cannot change. So I would say +1 to
 the idea of using `const` whenever it's possible and `let` otherwise.

 ----

 I read the article mcs mentioned, I think it has some interesting points.
 Giving my opinion about some of them:

 ...

 > It adds an extra distraction to the process of programming...

 I think this is true, but I also think that the additional distraction is
 quite small.

 ...
 > ...and results in code difficult to understand and change.

 I don't see how it's more difficult to understand, but I guess this is
 subjective. I think being able to easily see whether the value of a
 variable will remain the same for the whole function cannot make the code
 more difficult to understand. It probably does make the code more
 difficult to change, but not sure that's a bad an idea. You're changing a
 "this variable will never be reassigned" to "this variable might be
 reassigned later in this function (probably depending on some condition)".
 I think the small difficulty added by using `const` is justified here.

 ...

 > ... aggressive use of const devalues the operator

 The writer suggested usage of `const` is: `Constants should be declared at
 the top of modules and only in module scope`. I don't see how the value of
 those `const` declarations decreases by allowing other `const`
 declarations inside functions or blocks. I think it's quite easy to
 distinguish the case of "const declarations which are at the start of a
 file" from the rest `const` usages.

 ...

 > By const being the default declaration, let rises as the more visible
 style of declaration. The idea is that let flags where something funny is
 happening.

 I think the premise (`let` flags where something "funny" is happening) is
 wrong. It just means what it means, that this variable might be reassigned
 later in the function, (probably) depending on some condition.

 ...

 > However, function arguments like function(a,b,c) { are also allowed re-
 assignment, so it is a false sense of security to suggest no let means no
 funny business is happening.

 Again, I think no one is saying `no let means no funny business is
 happening` in the first place. From the point of view of what JavaScript
 allows, I think we should consider function arguments to be `implicit let
 declarations`, although I think it's bad practice to reassign them (and
 that can be enforced via style rules). But I don't see how this affects
 the `const` vs `let` discussion.

 ...

 > What is “expressed” by const itself when used this way? Since you are
 intended to refactor the declaration to let if the situation requires it,
 it can only express “this variable wasn’t being re-assigned when I wrote
 this code, but feel free to change that”. This is basically meaningless.

 I don't think `const` necessarily expresses "feel free to change that",
 and "when I wrote this code" is redundant. It just expresses "this
 variable is not reassigned".

 ...

 >Second, choosing const first really means choosing to think about every
 declaration. Will the next line of code change this assignment? How will I
 use this variable? Choosing let first eliminates this intellectual load,
 helping developers focus on more important problems.

 As I said before, it's hard to argue against the idea that there is "some"
 additional intellectual load. My opinion is that this added intellectual
 load when developing is fairly small, and that it's justified. I also
 think the examples chosen by the author do not do a good job at showcasing
 this added intellectual load. It should be quite obvious that a variable
 storing the array length before a loop should be `const`, and the variable
 of the classical `for` loop a `let`.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/26184#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online