#77 review changes

Signed-off-by: Chris Malley <cmalley@pixelzoom.com>
phetsims · Oct 4, 2018 · 7b63a08 · 7b63a08
1 parent e42277b
commit 7b63a08
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 32 deletions.
diff --git a/js/Range.js b/js/Range.js
@@ -1,4 +1,4 @@
-// Copyright 2013-2015, University of Colorado Boulder
+// Copyright 2013-2018, University of Colorado Boulder
 
 /**
  * A numeric range.
@@ -19,13 +19,13 @@ define( function( require ) {
    */
   function Range( min, max ) {
 
-    // @public (read-only) - the minimum value of the range
+    assert && assert( min <= max, 'max must be >= min. min: ' + min + ', max: ' + max );
+
+    // @private the minimum value of the range
     this._min = min;
 
-    // @public (read-only) - the maximum value of the range
+    // @private the maximum value of the range
     this._max = max;
-
-    assert && assert( min <= max, 'max must be greater than or equal to min. min: ' + min + ', max: ' + max );
   }
 
   dot.register( 'Range', Range );
@@ -52,7 +52,7 @@ define( function( require ) {
      * @param {number} min
      */
     setMin: function( min ) {
-      assert && assert( min <= this._max, 'min must be less than or equal to max: ' + min );
+      assert && assert( min <= this._max, 'min must be <= max: ' + min );
       this._min = min;
     },
     set min( min ) {
@@ -77,7 +77,7 @@ define( function( require ) {
      * @param {number} max
      */
     setMax: function( max ) {
-      assert && assert( this._min <= max, 'max must be greater than or equal to min: ' + max );
+      assert && assert( this._min <= max, 'max must be >= to min: ' + max );
       this._max = max;
     },
     set max( max ) {
@@ -91,7 +91,7 @@ define( function( require ) {
      * @param {number} max
      */
     setMinMax: function( min, max ) {
-      assert && assert( min <= max, 'max must be greater than or equal to min. min: ' + min + ', max: ' + max );
+      assert && assert( min <= max, 'max must be >= to min. min: ' + min + ', max: ' + max );
       this._min = min;
       this._max = max;
     },
@@ -140,7 +140,7 @@ define( function( require ) {
      * @returns {boolean}
      */
     containsRange: function( range ) {
-      return this._min <= range.min && this._max >= range.max;
+      return ( this._min <= range.min ) && ( this._max >= range.max );
     },
 
     /**
@@ -184,13 +184,13 @@ define( function( require ) {
     },
 
     /**
-     * Determines if this range is equal to other range.
+     * Determines if this Range is equal to some object.
      * @public
-     * @param {Range} other
+     * @param {*} object
      * @returns {boolean}
      */
-    equals: function( other ) {
-      return this._min === other.min && this._max === other.max;
+    equals: function( object ) {
+      return ( object instanceof Range ) && ( this._min === object.min ) && ( this._max === object.max );
     }
   };
 

diff --git a/js/RangeWithValue.js b/js/RangeWithValue.js
@@ -1,4 +1,4 @@
-// Copyright 2013-2015, University of Colorado Boulder
+// Copyright 2013-2018, University of Colorado Boulder
 
 /**
  * A numeric range with an optional default value.
@@ -19,17 +19,15 @@ define( function( require ) {
    * @param {number} [defaultValue] - default value inside the range
    * @constructor
    */
-
   function RangeWithValue( min, max, defaultValue ) {
 
-    Range.call( this, min, max );
+    defaultValue = ( defaultValue === undefined ) ? min : defaultValue;
+    assert && assert( defaultValue >= min && defaultValue <= max, 'defaultValue out of range: ' + defaultValue );
 
-    // @private - the default value of the range
-    this._defaultValue = ( defaultValue === undefined ) ? min : defaultValue;
+    Range.call( this, min, max );
 
-    assert && assert( this._defaultValue >= min && this._defaultValue <= max,
-      'Default value must be less than or equal to max and greater than or equal to min: ' + this._defaultValue
-    );
+    // @private
+    this._defaultValue = defaultValue;
   }
 
   dot.register( 'RangeWithValue', RangeWithValue );
@@ -38,7 +36,6 @@ define( function( require ) {
 
     /**
      * Getter for defaultValue
-     *
      * @returns {number}
      * @public
      */
@@ -56,7 +53,7 @@ define( function( require ) {
      * @override
      */
     setMin: function( min ) {
-      assert && assert( this._defaultValue >= min, 'min must be less than or equal to default value: ' + min );
+      assert && assert( this._defaultValue >= min, 'min must be <= defaultValue: ' + min );
       Range.prototype.setMin.call( this, min );
     },
 
@@ -67,7 +64,7 @@ define( function( require ) {
      * @override
      */
     setMax: function( max ) {
-      assert && assert( this._defaultValue <= max, 'max must be greater than or equal to default value: ' + max );
+      assert && assert( this._defaultValue <= max, 'max must be >= defaultValue: ' + max );
       Range.prototype.setMax.call( this, max );
     },
 
@@ -82,18 +79,16 @@ define( function( require ) {
     },
 
     /**
-     * Determines if this range is equal to other range.
-     * Note the default values must match as well.
+     * Determines if this RangeWithValue is equal to some object.
      * @public
-     * @param {Range} other
+     * @param {*} object
      * @returns {boolean}
      * @override
      */
-    equals: function( other ) {
-      return other instanceof RangeWithValue &&
-             this.min === other.min &&
-             this.max === other.max &&
-             this._defaultValue === other.defaultValue;
+    equals: function( object ) {
+      return ( object instanceof RangeWithValue ) &&
+             ( this._defaultValue === object.defaultValue ) &&
+             Range.prototype.equals.call( this, object );
     }
   } );
 } );