Skip to content

Commit

Permalink
refactor unnecessary class fields, #40
Browse files Browse the repository at this point in the history
  • Loading branch information
Hyodar committed Feb 25, 2020
1 parent bb62d70 commit 5db4d17
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 104 deletions.
6 changes: 3 additions & 3 deletions js/common/view/MassNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ define( require => {
// TODO - magic number
this.size = 20;

// @public {Property.<boolean>} 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;
} );

Expand Down Expand Up @@ -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' );
}
}

Expand Down
16 changes: 8 additions & 8 deletions js/common/view/SpringNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,27 @@ define( require => {
excludeInvisible: true
} );

// @public {Property.<boolean>} 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,
inputEnabled: false,
stroke: NormalModesColors.SPRING_STROKE,
lineWidth: 5
} );
this.addChild( this.line );
this.addChild( line );

let currentXScaling = 1;

Expand Down Expand Up @@ -83,7 +83,7 @@ define( require => {
}
} );

this.visibilityProperty.linkAttribute( this, 'visible' );
visibilityProperty.linkAttribute( this, 'visible' );
}
}

Expand Down
31 changes: 15 additions & 16 deletions js/one-dimension/view/MassNode1D.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 ) {
Expand All @@ -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;
Expand All @@ -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 );
}
}
} );
Expand Down
2 changes: 1 addition & 1 deletion js/one-dimension/view/ModeGraphCanvasNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ define( require => {
this.xStep = this.graphSize.width / this.curveResolution;

// @private {Array.<number>}
this.curveYPositions = new Array( this.curveResolution ); // @private
this.curveYPositions = new Array( this.curveResolution );

// @private {String} - curve stroke canvas color
this.strokeColor = options.strokeColor;
Expand Down
27 changes: 12 additions & 15 deletions js/one-dimension/view/OneDimensionScreenView.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
}

Expand Down
2 changes: 1 addition & 1 deletion js/one-dimension/view/StaticModeGraphCanvasNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 4 additions & 9 deletions js/one-dimension/view/WallNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) );
} );
}
}
Expand Down
3 changes: 2 additions & 1 deletion js/two-dimensions/TwoDimensionsConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
35 changes: 16 additions & 19 deletions js/two-dimensions/view/AmplitudeSelectorRectangle.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Loading

0 comments on commit 5db4d17

Please sign in to comment.