Skip to content

Commit

Permalink
review items complete for Charge.js, see #63
Browse files Browse the repository at this point in the history
  • Loading branch information
mbarlow12 committed Sep 21, 2018
1 parent 6dd8819 commit 02f6744
Showing 1 changed file with 13 additions and 29 deletions.
42 changes: 13 additions & 29 deletions js/common/model/Charge.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ define( function( require ) {
var DerivedPropertyIO = require( 'AXON/DerivedPropertyIO' );
var inherit = require( 'PHET_CORE/inherit' );
var ISLCObject = require( 'INVERSE_SQUARE_LAW_COMMON/model/ISLCObject' );
var Property = require( 'AXON/Property' );
var PropertyIO = require( 'AXON/PropertyIO' );

// ifphetio
var BooleanIO = require( 'ifphetio!PHET_IO/types/BooleanIO' );
var BooleanProperty = require( 'AXON/BooleanProperty' );

/**
* @param {number} initialCharge
Expand All @@ -37,23 +33,18 @@ define( function( require ) {
constantRadius: 6.75E-3 // ensure this is in meters (0.675cm)
}, options );

// REVIEW: Consider using BooleanProperty
var constantRadiusProperty = new Property( true, {
tandem: tandem.createTandem( 'constantRadiusProperty' ),
phetioType: PropertyIO( BooleanIO )
var constantRadiusProperty = new BooleanProperty( true, {
tandem: tandem.createTandem( 'constantRadiusProperty' )
} );

var negativeColor = new Color( '#00f' );
var positiveColor = new Color( '#f00' );

ISLCObject.call( this, initialCharge, initialPosition, valueRange, constantRadiusProperty, tandem, options );

// @public - object node color is will change with value
// color property will be updated based on a boolean value (negative vs positive)
// brightness will be set according to the Mass/Charge magnitude
// REVIEW: Use Property instead of property in above comment
// REVIEW: Visibility annotation? Type Doc?
// REVIEW: Should this be @protected visibility annotation? I may be wrong here.
// @public {Property.<Color>} - object node color is will change with value (linked in ISLCObjectNode.js)
// - color Property will be updated based on a boolean value (negative vs positive)
// - brightness will be set according to the Mass/Charge magnitude
this.baseColorProperty = new DerivedProperty( [ this.valueProperty ], function( value ) {
var newBaseColor = value < 0 ? negativeColor : positiveColor;
return newBaseColor.colorUtilsBrighter( 1 - Math.abs( value ) / valueRange.max );
Expand All @@ -66,21 +57,14 @@ define( function( require ) {

return inherit( ISLCObject, Charge, {

// @override
// Returns the {number} radius of the charge object
// REVIEW: jsDoc should have @return {type}. Eliminates need for {number} in function description.
// REVIEW: Function description should be first in jsDoc.
// REVIEW: Remove unused parameter?
calculateRadius: function( charge ) {
/**
* Returns the radius of the charge object.
*
* @override
* @return {number}
*/
calculateRadius: function() {
return this.radiusProperty.get();
},

// @public
// TODO: reset should be implemented in the parent class ISLCObject
// REVIEW: Handle review
reset: function() {
this.valueProperty.reset();
this.positionProperty.reset();
}
} );
} );

0 comments on commit 02f6744

Please sign in to comment.