lines should not exceed 120 columns

[pixelzoom]

Related to #2 (code review):

- [ ] Generally, lines should not exceed 120 columns. Break up long statements, expressions, or comments into multiple lines to optimize readability. It is OK for require statements or other structured patterns to exceed 120 columns. Use your judgment!

Example in Spring.js:

      // @public {Property.<boolean>} determines the visibility of the spring
      this.visibilityProperty = new DerivedProperty( [ this.leftMass.visibilityProperty, this.rightMass.visibilityProperty ], function( leftVisible, rightVisible ) {
        return leftVisible;
      } );

Here's one way of reformatting a DerivedProperty that works well:

      // @public {Property.<boolean>} determines the visibility of the spring
      this.visibilityProperty = new DerivedProperty(
        [ this.leftMass.visibilityProperty, this.rightMass.visibilityProperty ],
        function( leftVisible, rightVisible ) {
          return leftVisible;
        } );

I fixed the above example (and some others) as part of #4. But there may be some others that need to be addressed.

And I should emphasis that this is not a strict requirement. As the checklist item says, "use your judgement" to ensure that the code is easily readable.

[Hyodar]

In db9687f I fixed most of the lines that had more than 120 columns.

There is also something I noticed while working on TwoDimensionsModel.js. There are some really long variable names there, e. g. frequencyTimesAmplitudeXTimesSin and frequencySquaredTimesAmplitudeXTimesCos. I think these names should be reviewed, even though I don't have any ideas for them right now. @pixelzoom, maybe this should have its own issue?

These variables don't even need to be model attributes and can be replaced by const, something I'll address soon as #40.

[pixelzoom]

In db9687f I fixed most of the lines that had more than 120 columns.

Bravo!

... There are some really long variable names there, e. g. frequencyTimesAmplitudeXTimesSin and frequencySquaredTimesAmplitudeXTimesCos ...

I think those names are fine, especially since they are local variables. Though verbose, they are certainly descriptive, and I can't think of better names.

These variables don't even need to be model attributes and can be replaced by const, something I'll address soon as #40.

And yes, definitely change those local arrays in TwoDimensionsModel.js setExactPositions to be const.

[pixelzoom]

If the long names bother you (or anyone else :) you might add a comment above them like:

// The names of these arrays correspond to the formulas used to compute their values.
const amplitudeXTimesCos = [];
const amplitudeYTimesCos = [];
const frequencyTimesAmplitudeXTimesSin = [];
const frequencyTimesAmplitudeYTimesSin = [];
const frequencySquaredTimesAmplitudeXTimesCos = [];
const frequencySquaredTimesAmplitudeYTimesCos = [];

[pixelzoom]

Also in TwoDimensionsModel.js, I'd recommend replacing "Phs" abbreviation with "Phase" in these variable names:

frequencyTimesTimeMinusPhsX
frequencyTimesTimeMinusPhsY
frequencyTimesTimeMinusPhsXCos
frequencyTimesTimeMinusPhsYCos

The names are already long, and saving 2 characters is not worth the negative impact on readability.

[Hyodar]

If the long names bother you (or anyone else :) you might add a comment above them like ...

Good idea! I thought they might get a bit overwhelming for someone reading the function, but this should be enough to explain why they're named that way. Added it in 80e83d8.

Also in TwoDimensionsModel.js, I'd recommend replacing "Phs" abbreviation with "Phase" in these variable names ...

Yes yes, I remember noticing it but somehow didn't remember to replace those. Also done now.

[Hyodar]

In this last commit I just fixed a line that had more than 120 columns. When I was working on this issue some months ago I classified this one as a "reasonable" long line, but now that I've seen it again I thought it was worth splitting into multiple lines.

Anyway, these are the remaining lines that exceed 120 characters:

$ grep -r ".\{120\}"
two-dimensions/view/NormalModeAmplitudesAccordionBox.js:    const amplitudeDirectionRadioButtonGroup = new AmplitudeDirectionRadioButtonGroup( model.amplitudeDirectionProperty );
one-dimension/view/NormalModeSpectrumAccordionBox.js:    const amplitudeDirectionRadioButtonGroup = new AmplitudeDirectionRadioButtonGroup( model.amplitudeDirectionProperty );
one-dimension/model/OneDimensionModel.js:            assert( this.masses[ i ].velocityProperty.get().y === 0, 'bad result of recalculateVelocityAndAcceleration' );
one-dimension/model/OneDimensionModel.js:            assert( this.masses[ i ].accelerationProperty.get().y === 0, 'bad result of recalculateVelocityAndAcceleration' );
one-dimension/model/OneDimensionModel.js:            assert( this.masses[ i ].velocityProperty.get().x === 0, 'bad result of recalculateVelocityAndAcceleration' );
one-dimension/model/OneDimensionModel.js:            assert( this.masses[ i ].accelerationProperty.get().x === 0, 'bad result of recalculateVelocityAndAcceleration' );

In my opinion, those 6 lines are actually more easily readable this way. @pixelzoom, how do you feel about it? I'll assign it to you for review.

[pixelzoom]

Keep in mind that 120-chars is not a hard limit, it's up to developer discretion. And you're the developer, so it's up to you! But I agree, those lines are fine as is.

Assigning back to @Hyodar in case there's anything else to do for this issue. Feel free to close if you're done.

[Hyodar]

Keep in mind that 120-chars is not a hard limit, it's up to developer discretion. And you're the developer, so it's up to you!

Sure, that's why I ended up leaving those as they are now :)

So, if that's alright, I'll go ahead and close the issue.