Vote: underscores on variables to indicate privateness

[stacey-gammon]

Parts of our codebase contain variables or functions named with a preceding underscore to indicate they should not be accessed externally.
e.g.:

 class MyClass {
  constructor() {
    this._myPrivateVariable = "hi";
  }

  _myPrivateFunction() {
  ...
  }
 }

The airbnb style guide (which we never officially switched to, but unofficially lean towards) discourages this practice: https://github.com/airbnb/javascript#naming--leading-underscore

I personally don't think it will be needed once we switch to typescript which has the private keyword, but don't feel too strongly either way. I would however like to come to a consensus on this, I notice some team members use it consistently where other (like myself) don't use it at all.

cc @spalger

Thumbs up this issue if you wish to add an exception to our own style guide to continue the practice of using underscores. Thumbs down this issue if you wish to abide by the airbnb style guide and stop using underscores to indicate privateness

[spalger]

I think we should add an exception and encourage this pattern because:

• Disallowing the use of _ to designate private methods discourages people from breaking complicated logic into smaller/reusable methods
• We are all capable of controlling ourselves and not using _ prefixed methods
• The fact that this doesn't make things really private is irrelevant if we treat them as such
• This is a pattern that is very well established and used all over

[chrisronline]

Can someone provide some concrete examples of using this pattern effectively? I'm on the fence and am leaning towards just allowing it if others find it valuable, but would love to talk about this more with examples.

[azasypkin]

Regarding TS: when/if we migrate to TS, it will be easier to mark every underscore-prefixed method/property as private right away. Otherwise we'll have to scan the code to understand if it can be marked as private or not.

[stacey-gammon]

Really good point @azasypkin. I think I'll switch my vote given that benefit. Thanks for highlighting it!

[w33ble]

it will be easier to mark every underscore-prefixed method/property as private right away

The same is true once we have private fields in ecmascript, where we can change the _property to #property.

[spalger]

@chrisronline Perhaps the example that spurred this discussion is a decent one: https://github.com/spalger/kibana/blob/0d4e7e37ddf4e8871fabb24d14f0ff0d449e30f1/test/functional/services/test_subjects.js#L102

In that scenario, the testSubjects service is designed to provided an API for interacting with dom elements that hides the underlying Leaflet elements from the user (with the intention of limiting our exposure to the Leadfoot API). I don't want to expose testSubjects._mapAll() as a part of that API because it adds another method that is explicitly exposing Leaflet elements to the user. I do however want to reuse that functionality and benefit from it being a method of the service (like having access to this.findAll()).

This is a very simple example, but it's focused on the reason that one might want methods that are actually methods, but not a part of the "public" API