From 5db4d17b47b24884e860231b0af2815760cc8d7a Mon Sep 17 00:00:00 2001 From: Hyodar Date: Tue, 25 Feb 2020 17:57:57 -0300 Subject: [PATCH] refactor unnecessary class fields, #40 --- js/common/view/MassNode.js | 6 ++-- js/common/view/SpringNode.js | 16 ++++----- js/one-dimension/view/MassNode1D.js | 31 ++++++++-------- js/one-dimension/view/ModeGraphCanvasNode.js | 2 +- .../view/OneDimensionScreenView.js | 27 +++++++------- .../view/StaticModeGraphCanvasNode.js | 2 +- js/one-dimension/view/WallNode.js | 13 +++---- js/two-dimensions/TwoDimensionsConstants.js | 3 +- .../view/AmplitudeSelectorRectangle.js | 35 +++++++++---------- js/two-dimensions/view/MassNode2D.js | 31 ++++++++-------- .../view/TwoDimensionsScreenView.js | 27 +++++++------- 11 files changed, 89 insertions(+), 104 deletions(-) diff --git a/js/common/view/MassNode.js b/js/common/view/MassNode.js index 1d41d10..ae3a04e 100644 --- a/js/common/view/MassNode.js +++ b/js/common/view/MassNode.js @@ -31,9 +31,9 @@ define( require => { // TODO - magic number this.size = 20; - // @public {Property.} determines the visibility of the MassNode + // determines the visibility of the MassNode // TODO - this property is unnecessary (see https://github.com/phetsims/normal-modes/issues/45) - this.visibilityProperty = new DerivedProperty( [ mass.visibilityProperty ], function( massVisible ) { + const visibilityProperty = new DerivedProperty( [ mass.visibilityProperty ], function( massVisible ) { return massVisible; } ); @@ -70,7 +70,7 @@ define( require => { this.addChild( this.arrows.right ); this.addChild( this.arrows.bottom ); - this.visibilityProperty.linkAttribute( this, 'visible' ); + visibilityProperty.linkAttribute( this, 'visible' ); } } diff --git a/js/common/view/SpringNode.js b/js/common/view/SpringNode.js index 862dfd3..25b4d9f 100644 --- a/js/common/view/SpringNode.js +++ b/js/common/view/SpringNode.js @@ -34,19 +34,19 @@ define( require => { excludeInvisible: true } ); - // @public {Property.} determines the visibility of the SpringNode + // determines the visibility of the SpringNode // dispose is unnecessary because the SpringNode and the dependencies exist for the lifetime of the sim - this.visibilityProperty = new DerivedProperty( + const visibilityProperty = new DerivedProperty( [ spring.visibilityProperty, springsVisibilityProperty ], function( mySpringVisible, springsVisible ) { return mySpringVisible && springsVisible; } ); - // @private {Shape} shape of the spring path - this.springShape = new Shape().moveTo( 0, 0 ).lineTo( 1, 0 ); + // shape of the spring path + const springShape = new Shape().moveTo( 0, 0 ).lineTo( 1, 0 ); - // @private {Path} line path that represents a string - this.line = new Path( this.springShape, { + // line path that represents a string + const line = new Path( springShape, { preventFit: true, boundsMethod: 'none', pickable: false, @@ -54,7 +54,7 @@ define( require => { stroke: NormalModesColors.SPRING_STROKE, lineWidth: 5 } ); - this.addChild( this.line ); + this.addChild( line ); let currentXScaling = 1; @@ -83,7 +83,7 @@ define( require => { } } ); - this.visibilityProperty.linkAttribute( this, 'visible' ); + visibilityProperty.linkAttribute( this, 'visible' ); } } diff --git a/js/one-dimension/view/MassNode1D.js b/js/one-dimension/view/MassNode1D.js index ba409b3..d0e485f 100644 --- a/js/one-dimension/view/MassNode1D.js +++ b/js/one-dimension/view/MassNode1D.js @@ -31,8 +31,7 @@ define( require => { super( mass, modelViewTransform, tandem ); - // @public {Rectangle} - this.rect = new Rectangle( merge( { + const rect = new Rectangle( merge( { boundsMethod: 'unstroked', lineWidth: 4, rectWidth: this.size, @@ -41,13 +40,13 @@ define( require => { centerY: 0 }, NormalModesColors.MASS_COLORS ) ); - this.addChild( this.rect ); + this.addChild( rect ); - this.startCallback = ( event, listener ) => { + const startCallback = ( event, listener ) => { model.draggingMassIndexProperty.set( model.masses.indexOf( mass ) ); }; - this.dragCallback = ( event, listener ) => { + const dragCallback = ( event, listener ) => { model.arrowsVisibilityProperty.set( false ); const point = listener.modelPoint.minus( mass.equilibriumPositionProperty.get() ); if ( model.amplitudeDirectionProperty.get() === AmplitudeDirection.HORIZONTAL ) { @@ -60,12 +59,12 @@ define( require => { } }; - this.endCallback = ( event, listener ) => { + const endCallback = ( event, listener ) => { model.draggingMassIndexProperty.set( -1 ); model.computeModeAmplitudesAndPhases(); }; - this.overUpCallback = isOver => { + const overUpCallback = isOver => { const amplitudeDirection = model.amplitudeDirectionProperty.get(); if ( amplitudeDirection === AmplitudeDirection.VERTICAL ) { this.arrows.top.visible = isOver; @@ -77,29 +76,29 @@ define( require => { } }; - this.dragListener = new DragListener( { + const dragListener = new DragListener( { applyOffset: true, - start: this.startCallback, - drag: this.dragCallback, - end: this.endCallback, + start: startCallback, + drag: dragCallback, + end: endCallback, transform: modelViewTransform } ); - this.addInputListener( this.dragListener ); - const callback = this.overUpCallback.bind( this ); + this.addInputListener( dragListener ); + const callback = overUpCallback.bind( this ); // unlink is unnecessary, the MassNode1D and the dependency exists for the lifetime of the sim model.arrowsVisibilityProperty.link( arrowsVisible => { if ( arrowsVisible ) { // unlink is needed when the arrows become invisible - this.dragListener.isOverProperty.link( callback ); + dragListener.isOverProperty.link( callback ); } else { this.arrows.top.visible = false; this.arrows.bottom.visible = false; this.arrows.left.visible = false; this.arrows.right.visible = false; - if ( this.dragListener.isOverProperty.hasListener( callback ) ) { - this.dragListener.isOverProperty.unlink( callback ); + if ( dragListener.isOverProperty.hasListener( callback ) ) { + dragListener.isOverProperty.unlink( callback ); } } } ); diff --git a/js/one-dimension/view/ModeGraphCanvasNode.js b/js/one-dimension/view/ModeGraphCanvasNode.js index 6bdd146..f81995b 100644 --- a/js/one-dimension/view/ModeGraphCanvasNode.js +++ b/js/one-dimension/view/ModeGraphCanvasNode.js @@ -53,7 +53,7 @@ define( require => { this.xStep = this.graphSize.width / this.curveResolution; // @private {Array.} - this.curveYPositions = new Array( this.curveResolution ); // @private + this.curveYPositions = new Array( this.curveResolution ); // @private {String} - curve stroke canvas color this.strokeColor = options.strokeColor; diff --git a/js/one-dimension/view/OneDimensionScreenView.js b/js/one-dimension/view/OneDimensionScreenView.js index cb1a233..0ee089f 100644 --- a/js/one-dimension/view/OneDimensionScreenView.js +++ b/js/one-dimension/view/OneDimensionScreenView.js @@ -79,32 +79,29 @@ define( require => { // The springs are added first - // @private {SpringNode[]} Array that will contain all of the springNodes. - this.springNodes = model.springs.map( spring => { + model.springs.forEach( spring => { const springNode = new SpringNode( spring, modelViewTransform, model.springsVisibilityProperty, tandem.createTandem( 'springNodes' ) ); this.addChild( springNode ); - return springNode; } ); - this.leftWallNode = new WallNode( model.masses[ 0 ], modelViewTransform, tandem.createTandem( 'leftWallNode' ) ); - this.rightWallNode = new WallNode( model.masses[ model.masses.length - 1 ], modelViewTransform, tandem.createTandem( 'rightWallNode' ) ); + const leftWallNode = new WallNode( model.masses[ 0 ], modelViewTransform, tandem.createTandem( 'leftWallNode' ) ); + const rightWallNode = new WallNode( model.masses[ model.masses.length - 1 ], modelViewTransform, tandem.createTandem( 'rightWallNode' ) ); - this.addChild( this.leftWallNode ); - this.addChild( this.rightWallNode ); + this.addChild( leftWallNode ); + this.addChild( rightWallNode ); - // @private {MassNode[]} Array that will contain all of the massNodes. - this.massNodes = []; - for ( let i = 1; i < model.masses.length - 1; ++i ) { - this.massNodes.push( new MassNode1D( model.masses[ i ], modelViewTransform, model, tandem.createTandem( 'massNodes' ) ) ); - this.addChild( this.massNodes[ this.massNodes.length - 1 ] ); - } + // used slice to ignore the virtual stationary masses at the walls + model.masses.slice( 1, model.masses.length - 1 ).forEach( mass => { + const massNode = new MassNode1D( mass, modelViewTransform, model, tandem.createTandem( 'massNodes' ) ); + this.addChild( massNode ); + } ); - this.normalModesAccordionBox = new NormalModesAccordionBox( model, merge( { + const normalModesAccordionBox = new NormalModesAccordionBox( model, merge( { top: optionsPanel.bottom + 8, right: this.layoutBounds.maxX - OneDimensionConstants.SCREEN_VIEW_X_MARGIN - resetAllButton.width - 10 }, NormalModesColors.PANEL_COLORS ) ); - this.addChild( this.normalModesAccordionBox ); + this.addChild( normalModesAccordionBox ); } } diff --git a/js/one-dimension/view/StaticModeGraphCanvasNode.js b/js/one-dimension/view/StaticModeGraphCanvasNode.js index 8e5ebe9..76aca9e 100644 --- a/js/one-dimension/view/StaticModeGraphCanvasNode.js +++ b/js/one-dimension/view/StaticModeGraphCanvasNode.js @@ -55,7 +55,7 @@ define( require => { this.curveYPositions = new Array( this.curveResolution ); // @private {String} - curve stroke canvas color - this.strokeColor = options.strokeColor; // @private + this.strokeColor = options.strokeColor; // @private {String} - reference line (y = 0) stroke canvas color this.referenceLineStrokeColor = options.referenceLineStrokeColor; diff --git a/js/one-dimension/view/WallNode.js b/js/one-dimension/view/WallNode.js index da94877..94ea518 100644 --- a/js/one-dimension/view/WallNode.js +++ b/js/one-dimension/view/WallNode.js @@ -27,23 +27,18 @@ define( require => { constructor( mass, modelViewTransform, tandem ) { super( { cursor: 'pointer' } ); - // @private (read-only) Non-Property attributes - this.mass = mass; - this.modelViewTransform = modelViewTransform; - - // @public {Rectangle} - this.rect = new Rectangle( merge( { + const rect = new Rectangle( merge( { boundsMethod: 'unstroked', lineWidth: 2, rectWidth: 6, rectHeight: 80, cornerRadius: 2 }, NormalModesColors.WALL_COLORS ) ); - this.addChild( this.rect ); + this.addChild( rect ); // dispose is unnecessary, the WallNode and the dependencies exist for the lifetime of the sim - Property.multilink( [ this.mass.equilibriumPositionProperty, this.mass.displacementProperty ], ( massPosition, massDisplacement ) => { - this.translation = this.modelViewTransform.modelToViewPosition( massPosition.plus( massDisplacement ) ).subtract( new Vector2( this.rect.rectWidth / 2, this.rect.rectHeight / 2 ) ); + Property.multilink( [ mass.equilibriumPositionProperty, mass.displacementProperty ], ( massPosition, massDisplacement ) => { + this.translation = modelViewTransform.modelToViewPosition( massPosition.plus( massDisplacement ) ).subtract( new Vector2( rect.rectWidth / 2, rect.rectHeight / 2 ) ); } ); } } diff --git a/js/two-dimensions/TwoDimensionsConstants.js b/js/two-dimensions/TwoDimensionsConstants.js index c5841ab..6c5828b 100644 --- a/js/two-dimensions/TwoDimensionsConstants.js +++ b/js/two-dimensions/TwoDimensionsConstants.js @@ -3,7 +3,8 @@ /** * Constants used in multiple locations within this simulation. * - * @author UTFPR + * @author Thiago de Mendonça Mildemberger (UTFPR) + * @author Franco Barpp Gomes (UTFPR) */ define( require => { 'use strict'; diff --git a/js/two-dimensions/view/AmplitudeSelectorRectangle.js b/js/two-dimensions/view/AmplitudeSelectorRectangle.js index b5a61da..b67085e 100644 --- a/js/two-dimensions/view/AmplitudeSelectorRectangle.js +++ b/js/two-dimensions/view/AmplitudeSelectorRectangle.js @@ -65,30 +65,27 @@ define( require => { super( options ); - this.row = row; // @private - this.col = col; // @private + const backgroundRect = new Rectangle( options.backgroundRect ); + this.addChild( backgroundRect ); - this.backgroundRect = new Rectangle( options.backgroundRect ); - this.addChild( this.backgroundRect ); - - this.amplitudeChanged = ( amplitude, axis ) => { + const amplitudeChanged = ( amplitude, axis ) => { if ( model.amplitudeDirectionProperty.get() === axis ) { const maxAmp = maxAmpProperty.get(); const heightFactor = Math.min( 1, amplitude / maxAmp ); - this.backgroundRect.rectHeight = this.rectHeight * ( 1 - heightFactor ); + backgroundRect.rectHeight = this.rectHeight * ( 1 - heightFactor ); } }; - this.numMassesChanged = numMasses => { - if ( this.row < numMasses && this.col < numMasses ) { + const numMassesChanged = numMasses => { + if ( row < numMasses && col < numMasses ) { this.visible = true; this.rectWidth = this.rectHeight = options.rectGridSize * gridToRealSizeRatioProperty.get(); - this.backgroundRect.rectWidth = this.rectWidth; - this.amplitudeChanged( axisAmplitudesProperty.get()[ row ][ col ].get(), model.amplitudeDirectionProperty.get() ); + backgroundRect.rectWidth = this.rectWidth; + amplitudeChanged( axisAmplitudesProperty.get()[ row ][ col ].get(), model.amplitudeDirectionProperty.get() ); - const gridLeft = this.col * ( options.paddingGridSize + options.rectGridSize ); - const gridTop = this.row * ( options.paddingGridSize + options.rectGridSize ); + const gridLeft = col * ( options.paddingGridSize + options.rectGridSize ); + const gridTop = row * ( options.paddingGridSize + options.rectGridSize ); this.left = gridToRealSizeRatioProperty.get() * gridLeft; this.top = gridToRealSizeRatioProperty.get() * gridTop; @@ -98,25 +95,25 @@ define( require => { } }; - this.amplitudeDirectionChanged = amplitudeDirection => { + const amplitudeDirectionChanged = amplitudeDirection => { this.fill = ( amplitudeDirection === AmplitudeDirection.VERTICAL ) ? options.fillY : options.fillX; - this.amplitudeChanged( axisAmplitudesProperty.get()[ row ][ col ].get(), amplitudeDirection ); + amplitudeChanged( axisAmplitudesProperty.get()[ row ][ col ].get(), amplitudeDirection ); }; // unlink is unnecessary, exists for the lifetime of the sim model.modeXAmplitudeProperty[ row ][ col ].link( amplitude => { - this.amplitudeChanged( amplitude, AmplitudeDirection.HORIZONTAL ); + amplitudeChanged( amplitude, AmplitudeDirection.HORIZONTAL ); } ); // unlink is unnecessary, exists for the lifetime of the sim model.modeYAmplitudeProperty[ row ][ col ].link( amplitude => { - this.amplitudeChanged( amplitude, AmplitudeDirection.VERTICAL ); + amplitudeChanged( amplitude, AmplitudeDirection.VERTICAL ); } ); // unlink is unnecessary, exists for the lifetime of the sim - model.numVisibleMassesProperty.link( this.numMassesChanged ); + model.numVisibleMassesProperty.link( numMassesChanged ); // unlink is unnecessary, exists for the lifetime of the sim - model.amplitudeDirectionProperty.link( this.amplitudeDirectionChanged ); + model.amplitudeDirectionProperty.link( amplitudeDirectionChanged ); const isNear = function( n1, n2 ) { const EPS = 10e-5; diff --git a/js/two-dimensions/view/MassNode2D.js b/js/two-dimensions/view/MassNode2D.js index 791050e..f1809b1 100644 --- a/js/two-dimensions/view/MassNode2D.js +++ b/js/two-dimensions/view/MassNode2D.js @@ -30,21 +30,20 @@ define( require => { super( mass, modelViewTransform, tandem ); - // @public {Circle} - this.circle = new Circle( merge( { + const circle = new Circle( merge( { radius: this.size / 2, boundsMethod: 'unstroked', lineWidth: 4 }, NormalModesColors.MASS_COLORS ) ); - this.addChild( this.circle ); + this.addChild( circle ); const rotationPoint = new Vector2( 0, 0 ); for ( const arrow in this.arrows ) { this.arrows[ arrow ].rotateAround( rotationPoint, Math.PI / 4 ); } - this.startCallback = ( event, listener ) => { + const startCallback = ( event, listener ) => { let foundIndex = -1; let foundArray = null; for ( let i = 0; i < model.masses.length; i++ ) { @@ -61,46 +60,46 @@ define( require => { } ); }; - this.dragCallback = ( event, listener ) => { + const dragCallback = ( event, listener ) => { model.arrowsVisibilityProperty.set( false ); mass.displacementProperty.set( listener.modelPoint.minus( mass.equilibriumPositionProperty.get() ) ); }; - this.endCallback = ( event, listener ) => { + const endCallback = ( event, listener ) => { model.draggingMassIndexesProperty.set( null ); model.computeModeAmplitudesAndPhases(); }; - this.overUpCallback = isOver => { + const overUpCallback = isOver => { this.arrows.top.visible = isOver; this.arrows.bottom.visible = isOver; this.arrows.left.visible = isOver; this.arrows.right.visible = isOver; }; - this.dragListener = new DragListener( { + const dragListener = new DragListener( { applyOffset: true, - start: this.startCallback, - drag: this.dragCallback, - end: this.endCallback, + start: startCallback, + drag: dragCallback, + end: endCallback, transform: modelViewTransform } ); - this.addInputListener( this.dragListener ); - const callback = this.overUpCallback.bind( this ); + this.addInputListener( dragListener ); + const callback = overUpCallback.bind( this ); // unlink is unnecessary, the MassNode2D and the dependency exists for the lifetime of the sim model.arrowsVisibilityProperty.link( arrowsVisible => { if ( arrowsVisible ) { // unlink is needed when the arrows become invisible - this.dragListener.isOverProperty.link( callback ); + dragListener.isOverProperty.link( callback ); } else { this.arrows.top.visible = false; this.arrows.bottom.visible = false; this.arrows.left.visible = false; this.arrows.right.visible = false; - if ( this.dragListener.isOverProperty.hasListener( callback ) ) { - this.dragListener.isOverProperty.unlink( callback ); + if ( dragListener.isOverProperty.hasListener( callback ) ) { + dragListener.isOverProperty.unlink( callback ); } } } ); diff --git a/js/two-dimensions/view/TwoDimensionsScreenView.js b/js/two-dimensions/view/TwoDimensionsScreenView.js index 6892fdd..e00f389 100644 --- a/js/two-dimensions/view/TwoDimensionsScreenView.js +++ b/js/two-dimensions/view/TwoDimensionsScreenView.js @@ -67,21 +67,17 @@ define( require => { this.addChild( optionsPanel ); this.addChild( resetAllButton ); - // @private {SpringNode[]} Array that will contain all of the X axis springNodes. - this.springXNodes = model.springsX.map( springArray => { - return springArray.map( spring => { + model.springsX.forEach( springArray => { + springArray.forEach( spring => { const springNode = new SpringNode( spring, modelViewTransform, model.springsVisibilityProperty, tandem.createTandem( 'springNodes' ) ); this.addChild( springNode ); - return springNode; } ); } ); - // @private {SpringNode[]} Array that will contain all of the Y axis springNodes. - this.springYNodes = model.springsY.map( springArray => { - return springArray.map( spring => { + model.springsY.forEach( springArray => { + springArray.forEach( spring => { const springNode = new SpringNode( spring, modelViewTransform, model.springsVisibilityProperty, tandem.createTandem( 'springNodes' ) ); this.addChild( springNode ); - return springNode; } ); } ); @@ -106,13 +102,14 @@ define( require => { this.addChild( normalModeAmplitudesAccordionBox ); - this.massNodes = []; - for ( let i = 1; i < model.masses.length - 1; ++i ) { - for ( let j = 1; j < model.masses[ i ].length - 1; ++j ) { - this.massNodes.push( new MassNode2D( model.masses[ i ][ j ], modelViewTransform, model, tandem.createTandem( 'massNodes' ) ) ); - this.addChild( this.massNodes[ this.massNodes.length - 1 ] ); - } - } + // used slice to ignore the virtual stationary masses at the walls + model.masses.slice( 1, model.masses.length - 1 ).forEach( massArray => { + massArray.slice( 1, massArray.length - 1 ).forEach( mass => { + const massNode = new MassNode2D( mass, modelViewTransform, model, tandem.createTandem( 'massNodes' ) ); + this.addChild( massNode ); + } ); + } ); + } }