simplify toScientificNotation by deleting special cases that are hand…

…led by the general case; delete workaround for toFixed; adding unit tests; #747
phetsims · Jun 27, 2022 · 5a03225 · 5a03225
1 parent afe8bfa
commit 5a03225
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 59 deletions.
diff --git a/js/ScientificNotationNode.ts b/js/ScientificNotationNode.ts
@@ -152,8 +152,8 @@ export default class ScientificNotationNode extends Node {
     }
     else {
       const scientificNotation = ScientificNotationNode.toScientificNotation( value, options );
-      const mantissaNumber = Utils.toFixedNumber( Number( scientificNotation.mantissa ), options.mantissaDecimalPlaces );
-      const exponentNumber = Number( scientificNotation.exponent );
+      const mantissaNumber = Utils.toFixedNumber( parseFloat( scientificNotation.mantissa ), options.mantissaDecimalPlaces );
+      const exponentNumber = parseInt( scientificNotation.exponent, 10 );
 
       if ( mantissaNumber === 0 && options.showZeroAsInteger ) {
 
@@ -193,67 +193,36 @@ export default class ScientificNotationNode extends Node {
       exponent: null // exponent will be computed
     }, providedOptions );
 
-    let mantissa: number;
-    let exponent: number;
-    if ( value === 0 ) {
-      mantissa = 0;
-      if ( options.exponent === null ) {
-
-        // 0 is represented as 0 x 10^0
-        exponent = 0;
-      }
-      else {
-
-        // 0 is represented as 0 x 10^E, where E is options.exponent
-        exponent = options.exponent;
-      }
-    }
-    else if ( options.exponent === 0 ) {
-
-      // M x 10^0
-      mantissa = Utils.toFixedNumber( value, options.mantissaDecimalPlaces );
-      exponent = 0;
-    }
-    else if ( options.exponent !== null ) {
+    let mantissa: string;
+    let exponent: string;
+    if ( options.exponent !== null ) {
 
       // M x 10^E, where E is options.exponent
-      mantissa = Utils.toFixedNumber( value / Math.pow( 10, options.exponent ), options.mantissaDecimalPlaces );
-      exponent = options.exponent;
+      //TODO https://github.com/phetsims/dot/issues/113 Utils.toFixed is subject to floating-point error
+      mantissa = Utils.toFixed( value / Math.pow( 10, options.exponent ), options.mantissaDecimalPlaces );
+      exponent = options.exponent.toString();
     }
     else {
 
-      // Convert to a string in exponential notation (eg 2e+2).
-      // Request an additional decimal place, because toExponential uses toFixed, which doesn't round the same on all platforms.
-      const exponentialString = value.toExponential( options.mantissaDecimalPlaces + 1 );
+      // Convert to a string in exponential notation, where the mantissa has 1 digit to the left of the decimal place.
+      //TODO https://github.com/phetsims/scenery-phet/issues/613 toExponential uses Number.toFixed, which doesn't round the same on all platforms
+      const exponentialString = value.toExponential( options.mantissaDecimalPlaces );
 
       // Break into mantissa and exponent tokens.
       const tokens = exponentialString.toLowerCase().split( 'e' );
+      mantissa = tokens[ 0 ];
+      exponent = tokens[ 1 ];
 
-      // Adjust the mantissa token to the correct number of decimal places, using nearest-neighbor rounding.
-      mantissa = Utils.toFixedNumber( parseFloat( tokens[ 0 ] ), options.mantissaDecimalPlaces );
-      exponent = Number( tokens[ 1 ] );
-
-      // If the mantissa is exactly 10, shift that power of 10 to the exponent.
-      // See https://github.com/phetsims/scenery-phet/issues/613
-      if ( mantissa === 10 ) {
-        mantissa = 1;
-        exponent += 1;
-      }
-
-      // Convert if a specific exponent was requested.
-      if ( options.exponent !== null ) {
-        mantissa = Utils.toFixedNumber( mantissa * Math.pow( 10, exponent - options.exponent ),
-          Math.max( 0, options.mantissaDecimalPlaces ) );
-        exponent = options.exponent;
+      // Remove the sign from the exponent.
+      if ( exponent[ 0 ] === '+' || exponent[ 0 ] === '-' ) {
+        exponent = exponent.substring( 1, exponent.length );
       }
     }
 
     // mantissa x 10^exponent
     return {
-
-      // restore precision, in case toFixedNumber removed zeros to right of the decimal point
-      mantissa: Utils.toFixed( mantissa, options.mantissaDecimalPlaces ),
-      exponent: exponent.toString()
+      mantissa: mantissa,
+      exponent: exponent
     };
   }
 }

diff --git a/js/ScientificNotationNodeTests.js b/js/ScientificNotationNodeTests.js
@@ -92,25 +92,31 @@ QUnit.test( 'exponent !== 0 && exponent !== null', assert => {
  */
 QUnit.test( 'rounding', assert => {
 
-  // This case was reported in https://github.com/phetsims/build-a-nucleus/issues/24 with bad mantissa 4.3
-  // assert.deepEqual( ScientificNotationNode.toScientificNotation( 424.8, {
-  //   exponent: null,
-  //   mantissaDecimalPlaces: 1
-  // } ), { mantissa: '4.2', exponent: '2' } );
+  // This case was reported in https://github.com/phetsims/build-a-nucleus/issues/24 with bad mantissa '4.3'
+  assert.deepEqual( ScientificNotationNode.toScientificNotation( 424.8, {
+    exponent: null,
+    mantissaDecimalPlaces: 1
+  } ), { mantissa: '4.2', exponent: '2' } );
 
   assert.deepEqual( ScientificNotationNode.toScientificNotation( 425.8, {
     exponent: null,
     mantissaDecimalPlaces: 2
   } ), { mantissa: '4.26', exponent: '2' } );
 
-  // This case was reported in https://github.com/phetsims/build-a-nucleus/issues/24, with bad mantissa 9.7
-  // assert.deepEqual( ScientificNotationNode.toScientificNotation( 9648, {
-  //   exponent: null,
-  //   mantissaDecimalPlaces: 1
-  // } ), { mantissa: '9.6', exponent: '3' } );
+  // This case was reported in https://github.com/phetsims/build-a-nucleus/issues/24, with bad mantissa '9.7'
+  assert.deepEqual( ScientificNotationNode.toScientificNotation( 9648, {
+    exponent: null,
+    mantissaDecimalPlaces: 1
+  } ), { mantissa: '9.6', exponent: '3' } );
 
   assert.deepEqual( ScientificNotationNode.toScientificNotation( 9658, {
     exponent: null,
     mantissaDecimalPlaces: 2
   } ), { mantissa: '9.66', exponent: '3' } );
+
+  // This case was reported in https://github.com/phetsims/scenery-phet/issues/747, with bad mantissa '35.85'
+  // assert.deepEqual( ScientificNotationNode.toScientificNotation( 35.855, {
+  //   exponent: 0,
+  //   mantissaDecimalPlaces: 2
+  // } ), { mantissa: '35.86', exponent: '0' } );
 } );