adapted to PhET coding style, #51

js/ohms-law/OhmsLawConstants.js

@@ -15,9 +15,6 @@ define( function( require ) {
 
   var OhmsLawConstants = {
 
-    // constants
-    AA_VOLTAGE: 1.5, // in volts
-
     // colors
     BLUE_COLOR: '#0f0ffb',
     BLACK_COLOR: '#000',
@@ -48,7 +45,8 @@ define( function( require ) {
 
     // battery
     BATTERY_WIDTH: 82,
-    BATTERY_HEIGHT: 40
+    BATTERY_HEIGHT: 40,
+    AA_VOLTAGE: 1.5 // in volts
   };
 
   ohmsLaw.register( 'OhmsLawConstants', OhmsLawConstants );

js/ohms-law/model/OhmsLawModel.js

@@ -1,7 +1,7 @@
-// Copyright 2013-2015, University of Colorado Boulder
+// Copyright 2013-2017, University of Colorado Boulder
 
 /**
- * Primary model for the Ohm's Law simulation
+ * Primary model for the Ohm's Law simulation, see doc/model.md for more information.
  * @author Vasily Shakhov (Mlearner)
  * @author Anton Ulyanov (Mlearner)
  */

js/ohms-law/view/BatteriesView.js

@@ -17,40 +17,46 @@ define( function( require ) {
 
   /**
    * @param {Property.<number>} voltageProperty
+   * @param {Object} options
    * @constructor
    */
-  function BatteriesView( voltageProperty ) {
+  function BatteriesView( voltageProperty, options ) {
     Node.call( this );
 
-    // max number of batteries
+    // Max number of batteries
     var maxNumberBatteries = Math.ceil( OhmsLawConstants.VOLTAGE_RANGE.max / OhmsLawConstants.AA_VOLTAGE );
 
-    // store battery nodes in an array
+    // Store battery nodes in an array
     var batteries = [];
 
-    // create an array of batteries
+    // Create an array of batteries; enough to fill the entire wire.
     for ( var i = 0; i < maxNumberBatteries; i++ ) {
       var leftPosition = i * OhmsLawConstants.BATTERY_WIDTH;
       var battery = new BatteryView( { x: leftPosition, y: 0 } );
+
+      // Add them as children to this node, and to the array for manipulation
       this.addChild( battery );
       batteries.push( battery );
     }
 
-    // present for the lifetime of the simulation
+    // Present for the lifetime of the simulation
     voltageProperty.link( function( voltage ) {
 
       batteries.forEach( function( battery, index ) {
-        // determine associated with a particular battery
+
+        // Determine associated with a particular battery
         var voltageBattery = Math.min( OhmsLawConstants.AA_VOLTAGE, voltage - index * OhmsLawConstants.AA_VOLTAGE );
 
-        // set the visibility of the battery
+        // Battery is only visible if it has a voltage.
         battery.visible = ( voltageBattery > 0 );
 
         if ( battery.visible ) {
           battery.setVoltage( voltageBattery );
         }
       } );
     } );
+
+    this.mutate( options );
   }
 
   ohmsLaw.register( 'BatteriesView', BatteriesView );

js/ohms-law/view/BatteryView.js

@@ -31,6 +31,19 @@ define( function( require ) {
   var COPPER_PORTION_WIDTH = OhmsLawConstants.BATTERY_WIDTH - MAIN_BODY_WIDTH - NUB_WIDTH;
   var BATTERY_HEIGHT = OhmsLawConstants.BATTERY_HEIGHT;
   var NUB_HEIGHT = OhmsLawConstants.BATTERY_HEIGHT * 0.30;
+  var VOLTAGE_TO_SCALE = new LinearFunction( 0.1, OhmsLawConstants.AA_VOLTAGE, 0.0001, 1, true );  // convert voltage to percentage (0 to 1)
+  var VOLTAGE_STRING_MAX_WIDTH = new Text( '9.9', { font: FONT } ).width;
+
+  // Fills for the battery
+  var MAIN_BODY_FILL = new LinearGradient( 0, 0, 0, BATTERY_HEIGHT )
+    .addColorStop( 0, '#777777' )
+    .addColorStop( 0.3, '#bdbdbd' )
+    .addColorStop( 1, '#2b2b2b' );
+  var COPPER_PORTION_FILL = new LinearGradient( 0, 0, 0, BATTERY_HEIGHT )
+    .addColorStop( 0, '#cc4e00' )
+    .addColorStop( 0.3, '#dddad6' )
+    .addColorStop( 1, '#cc4e00' );
+  var NUB_FILL = '#dddddd';
 
   /**
    * @param {Object} [options]
@@ -40,90 +53,86 @@ define( function( require ) {
 
     Node.call( this );
 
-    // convert voltage to percentage (0 to 1)
-    var voltageToScale = new LinearFunction( 0.1, OhmsLawConstants.AA_VOLTAGE, 0.0001, 1, true );
-
-    // fill for the battery
-    var mainBodyFill = new LinearGradient( 0, 0, 0, BATTERY_HEIGHT )
-      .addColorStop( 0, '#777777' )
-      .addColorStop( 0.3, '#bdbdbd' )
-      .addColorStop( 1, '#2b2b2b' );
-    var copperPortionFill = new LinearGradient( 0, 0, 0, BATTERY_HEIGHT )
-      .addColorStop( 0, '#cc4e00' )
-      .addColorStop( 0.3, '#dddad6' )
-      .addColorStop( 1, '#cc4e00' );
-    var nubFill = '#dddddd';
-
-    // the origin (0,0) is defined as the leftmost and vertically centered position of the battery
+    // The origin (0,0) is defined as the leftmost and vertically centered position of the battery
     var batteryNode = new Node();
-    var mainBody = new Rectangle( 0, 0, MAIN_BODY_WIDTH, BATTERY_HEIGHT, {
+
+    // @private
+    this.mainBody = new Rectangle( 0, 0, MAIN_BODY_WIDTH, BATTERY_HEIGHT, {
       stroke: '#000',
       lineWidth: 1,
-      fill: mainBodyFill,
+      fill: MAIN_BODY_FILL,
       y: -BATTERY_HEIGHT / 2
     } );
-    var copperPortion = new Rectangle( 0, 0, COPPER_PORTION_WIDTH, BATTERY_HEIGHT, {
+    batteryNode.addChild( this.mainBody );
+
+    // @private
+    this.copperPortion = new Rectangle( 0, 0, COPPER_PORTION_WIDTH, BATTERY_HEIGHT, {
       stroke: '#000',
       lineWidth: 1,
-      fill: copperPortionFill,
+      fill: COPPER_PORTION_FILL,
       y: -BATTERY_HEIGHT / 2,
       x: MAIN_BODY_WIDTH
     } );
-    var nub = new Rectangle( COPPER_PORTION_WIDTH, 0, NUB_WIDTH, NUB_HEIGHT, {
+    batteryNode.addChild( this.copperPortion );
+
+    // @private
+    this.nub = new Rectangle( COPPER_PORTION_WIDTH, 0, NUB_WIDTH, NUB_HEIGHT, {
       stroke: '#000',
       lineWidth: 1,
-      fill: nubFill,
+      fill: NUB_FILL,
       y: -NUB_HEIGHT / 2,
       x: MAIN_BODY_WIDTH
     } );
-    // add battery to the scene graph
-    batteryNode.addChild( mainBody );
-    batteryNode.addChild( copperPortion );
-    batteryNode.addChild( nub );
+    batteryNode.addChild( this.nub );
+
     this.addChild( batteryNode );
 
-    // voltage label associated with the battery
-    var batteryText = new Node( { x: 3 } );
-    var voltageStringMaxWidth = new Text( '9.9', { font: FONT } ).width;
-    var voltageValueText = new Text( OhmsLawConstants.AA_VOLTAGE, { font: FONT } );
+    // @private - Voltage label associated with the battery
+    this.batteryText = new Node( { x: 3 } );
+
+    // @private
+    this.voltageValueText = new Text( OhmsLawConstants.AA_VOLTAGE, { font: FONT } );
+    this.batteryText.addChild( this.voltageValueText );
+
     var voltageUnitsText = new Text( voltageUnitsString, {
       font: FONT,
       fill: 'blue',
-      x: voltageStringMaxWidth * 1.1,
-      maxWidth: ( MAIN_BODY_WIDTH - voltageStringMaxWidth ) * 0.9 // limit to 90% of remaining space
+      x: VOLTAGE_STRING_MAX_WIDTH * 1.1,
+      maxWidth: ( MAIN_BODY_WIDTH - VOLTAGE_STRING_MAX_WIDTH ) * 0.9 // limit to 90% of remaining space
     } );
-    this.addChild( batteryText );
-    batteryText.addChild( voltageValueText );
-    batteryText.addChild( voltageUnitsText );
+    this.batteryText.addChild( voltageUnitsText );
+
+    this.addChild( this.batteryText );
 
     this.mutate( options );
+  }
+
+  ohmsLaw.register( 'BatteryView', BatteryView );
+
+  return inherit( Node, BatteryView,{
 
     /**
-     * set the length of the battery as well as voltage text and position of the text associated with the battery
+     * Set the length of the battery as well as voltage text and position of the text associated with the battery
      * @param {number} voltage
+     * @public
      */
-    this.setVoltage = function( voltage ) {
+    setVoltage: function( voltage ) {
       // update the text of the
-      voltageValueText.text = Util.toFixed( voltage, 1 );
+      this.voltageValueText.text = Util.toFixed( voltage, 1 );
 
       // adjust length of the battery
-      mainBody.setRect( 0, 0, MAIN_BODY_WIDTH * voltageToScale( voltage ), BATTERY_HEIGHT );
-      copperPortion.x = mainBody.right;
-      nub.x = mainBody.right;
+      this.mainBody.setRect( 0, 0, MAIN_BODY_WIDTH * VOLTAGE_TO_SCALE( voltage ), BATTERY_HEIGHT );
+      this.copperPortion.x = this.mainBody.right;
+      this.nub.x = this.mainBody.right;
 
       // set vertical position of the voltage label
       if ( voltage >= OhmsLawConstants.AA_VOLTAGE ) {
-        batteryText.centerY = -7; // move slightly up from centered position
+        this.batteryText.centerY = -7; // move slightly up from centered position
       }
       // move up if the voltage is greater than 0.1 but less than OhmsLawConstants.AA_VOLTAGE
       else if ( voltage >= 0.1 ) {
-        batteryText.centerY = -BATTERY_HEIGHT / 2 - 12; // place it above the battery
+        this.batteryText.centerY = -BATTERY_HEIGHT / 2 - 12; // place it above the battery
       }
-
-    };
-  }
-
-  ohmsLaw.register( 'BatteryView', BatteryView );
-
-  return inherit( Node, BatteryView );
+    }
+  } );
 } );

js/ohms-law/view/ControlPanel.js

@@ -31,35 +31,34 @@ define( function( require ) {
    */
   function ControlPanel( voltageProperty, resistanceProperty ) {
 
-    // create the voltage slider with readout and labels
+    // Create the voltage slider with readout and labels
     var voltageSlider = new SliderUnit(
       voltageProperty,
       OhmsLawConstants.VOLTAGE_RANGE,
       voltageSymbolString,
       voltageString,
       voltageUnitsString, {
-        keyboardStep: 0.1
+        keyboardStep: 0.1,
+        centerX: -OhmsLawConstants.SLIDERS_HORIZONTAL_SEPARATION / 2
       } );
 
-    // create the resistance slider with readout and labels
+    // Create the resistance slider with readout and labels
     var resistanceSlider = new SliderUnit(
       resistanceProperty,
       OhmsLawConstants.RESISTANCE_RANGE,
       resistanceSymbolString,
       resistanceString,
       resistanceUnitsString, {
         numberDecimalPlaces: 0,
-        keyboardStep: 5
+        keyboardStep: 5,
+        centerX: voltageSlider.centerX + OhmsLawConstants.SLIDERS_HORIZONTAL_SEPARATION
        } );
 
+    // Use a content node so that the Panel can surround it fully
     var content = new Node();
     content.addChild( voltageSlider );
     content.addChild( resistanceSlider );
 
-    // set the horizontal position of the sliders, defining the middle slider as zero
-    voltageSlider.centerX = -OhmsLawConstants.SLIDERS_HORIZONTAL_SEPARATION / 2;
-    resistanceSlider.centerX = voltageSlider.centerX + OhmsLawConstants.SLIDERS_HORIZONTAL_SEPARATION;
-
     Panel.call( this, content, {
       xMargin: 30,
       yMargin: 20,

js/ohms-law/view/FormulaNode.js

@@ -1,4 +1,4 @@
-// Copyright 2016, University of Colorado Boulder
+// Copyright 2016-2017, University of Colorado Boulder
 
 /**
  * view formula ohms law
@@ -24,17 +24,23 @@ define( function( require ) {
   var currentSymbolString = require( 'string!OHMS_LAW/currentSymbol' );
   var resistanceSymbolString = require( 'string!OHMS_LAW/resistanceSymbol' );
 
+  // constants
+  // Center Y position of all text in the node, empirically determined
+  var CENTER_Y = 160;
+
+
   /**
+   * @param {Property.<number>} currentProperty
    * @param {Property.<number>} voltageProperty
    * @param {Property.<number>} resistanceProperty
-   * @param {Property.<number>} currentProperty
    * @constructor
    */
-  function FormulaNode( voltageProperty, resistanceProperty, currentProperty ) {
+  function FormulaNode( currentProperty, voltageProperty, resistanceProperty ) {
 
     Node.call( this );
 
-    var texts = [
+    // Hold the metaData for each text to be created
+    var textsDataArray = [
       {
         symbolString: currentSymbolString,
         scaleA: 0.2,
@@ -64,57 +70,56 @@ define( function( require ) {
       }
     ];
 
-    // create a node to hold all the symbols
+    // Create a node to hold all the symbols
     var lettersNode = new Node();
     this.addChild( lettersNode );
 
-    // center Y position of all text in the node, empirically determined
-    var centerY = 160;
-
-    // add the equals sign, which does not change size
-    var equalsSign = new Text( '=', {
+    // Add the equals sign, which does not change size
+    var equalsSign = new Text( '=', { // We never internationalize the '=' sign
       font: new PhetFont( { family: OhmsLawConstants.FONT_FAMILY, size: 140, weight: 'bold' } ),
       fill: '#000',
       centerX: 300,
-      centerY: centerY
+      centerY: CENTER_Y
     } );
     this.addChild( equalsSign ); // must come after lettersNode
 
-    // add the symbol letters to the formula and scale them appropriately
-    texts.forEach( function( entry ) {
+    // Add the symbol letters to the formula and scale them appropriately
+    textsDataArray.forEach( function( textData ) {
 
-      // centered text node, so we just have to adjust scale dynamically
-      var textNode = new Text( entry.symbolString, {
+      // Centered text node, so we just have to adjust scale dynamically
+      var textNode = new Text( textData.symbolString, {
         font: new PhetFont( { family: OhmsLawConstants.FONT_FAMILY, size: 20, weight: 'bold' } ),
-        fill: entry.color,
+        fill: textData.color,
         centerX: 0,
         centerY: 0
       } );
 
       // Make sure that the text isn't initially too large and, if so, change the scaling factors.  This is done in
       // support of translation, in case some symbols are much larger than the V, I, and R symbols used in the English
       // version.
-      var initialWidth = textNode.width * entry.scaleA * entry.property.value + entry.scaleB;
-      if ( initialWidth > entry.maxInitialWidth ) {
-        var scaleFactor = entry.maxInitialWidth / initialWidth;
-        entry.scaleA = entry.scaleA * scaleFactor;
-        entry.scaleB = entry.scaleB * scaleFactor;
+      var initialWidth = textNode.width * textData.scaleA * textData.property.value + textData.scaleB;
+      if ( initialWidth > textData.maxInitialWidth ) {
+        var scaleFactor = textData.maxInitialWidth / initialWidth;
+        textData.scaleA = textData.scaleA * scaleFactor;
+        textData.scaleB = textData.scaleB * scaleFactor;
       }
 
-      // add an invisible rectangle with bounds slightly larger than the text so that artifacts aren't left on the
+      // Add an invisible rectangle with bounds slightly larger than the text so that artifacts aren't left on the
       // screen, see https://github.com/phetsims/ohms-law/issues/26.
       var antiArtifactRectangle = Rectangle.bounds( textNode.bounds.dilatedX( 1 ), { fill: 'rgba( 0, 0, 0, 0 )' } );
 
-      // create the node that contains the text
+      // Create the node that contains the text
       var letterNode = new Node( { children: [ antiArtifactRectangle, textNode ] } );
       lettersNode.addChild( letterNode );
 
-      // scale the text as the associated value changes
-      entry.property.link( function updateProperty( value ) {
-        // performance TODO: consider not updating the matrix if it hasn't changed (if entry.x, entry.scaleA, and entry.scaleB haven't changed)
-        // since it would potentially reduce the area of SVG that gets repainted (may be browser-specific)
-        letterNode.matrix = Matrix3.translation( entry.x, centerY )
-          .timesMatrix( Matrix3.scale( entry.scaleA * value + entry.scaleB ) );
+      // Scale the text as the associated value changes
+      textData.property.link( function updateProperty( value ) {
+
+        // Since it would potentially reduce the area of SVG that gets repainted (may be browser-specific)
+        letterNode.matrix = Matrix3.translation( textData.x, CENTER_Y )
+          .timesMatrix( Matrix3.scale( textData.scaleA * value + textData.scaleB ) );
+
+        // TODO: Performance: consider not updating the matrix if it hasn't changed (if textData.x, textData.scaleA, and textData.scaleB haven't changed)
       } );
     } );
   }