Inline compareTo for numbers into min and max.

corelib/src/math.dart

Index: corelib/src/math.dart
diff --git a/corelib/src/math.dart b/corelib/src/math.dart
index 1acd20df3c724f3c71902fdba37eeb5341055b1f..b6127a14deb17fcf92b36691c4a1e64323189d2c 100644
--- a/corelib/src/math.dart
+++ b/corelib/src/math.dart
@@ -60,20 +60,59 @@ class Math {
   static double parseDouble(String str) => MathNatives.parseDouble(str);
 
   static num min(num a, num b) {
-    int c = a.compareTo(b);
-    if (c == 0) return a;
-    if (c < 0) {
-      if ((b is double) && b.isNaN()) return b;
-      return a;
+    if (a is num) {  // TODO(floitsch): move this into one if.

[sra1, 2012/07/04 19:28:23]

Under what conditions? (description or issue #)
Presumably something undesirable happens if you do it now.

[floitsch, 2012/07/05 17:48:49]

Done.

+      if (b is num) {
+        // Special case for NaN and -0.0. If one argument is NaN return NaN.
+        // In the context of [min] we have furthermore -0.0 < 0.0.

[sra1, 2012/07/04 19:28:23]

What does 'in the context of [min]' mean here, in the body of [min]?

[floitsch, 2012/07/05 17:48:49]

Done.

+        if (a > b) return b;
+        if (a == b) {
+          if (a is double && a == 0.0) {

[Lasse Reichstein Nielsen, 2012/07/05 09:34:41]

This is a little tricky, since in dart2js 0.0 is both int and double.
If you wrote it as the, in pure Dart equivalent, 'a is! int' (since num has only
int and double as subclasses) it won't work. This might warrant a comment,.

[floitsch, 2012/07/05 17:48:49]

removed with your code.

+            if (b is double) {
+              if (1.0 / a < 0.0) {

[Lasse Reichstein Nielsen, 2012/07/05 09:34:41]

Consider checking as
  1.0 / a == -double.Infinity
instead. It's possible to do as a simple bit-wise comparison instead of doing
real computations in a non-finite value. That's usually very slow.

Or just replace this all with 
 if (a == b) {
   if (a == 0) { 
     // a and b are both either 0, 0.0 or -0.0
     // The following returns -0 if either a or b is -0,
     // and it returns an int if they are both int 0.
     return (a + b) * a * b;  
   }
   return a; 
 }

[floitsch, 2012/07/05 17:48:49]

nice.
done.

+                // a == -0.0.
+                return a;
+              }
+              // b == -0.0 or 0.0.
+              return b;
+            }
+            // a == -0.0 or 0.0, and b == 0.
+            return a;
+          }
+          return b;
+        }
+        if (a != a) return a;
+        return b;
+      }
     }
-    if ((a is double) && a.isNaN()) return a;
-    return b;
+    return a.compareTo(b) <= 0 ? a : b;

[Lasse Reichstein Nielsen, 2012/07/05 09:34:41]

Consider using (0 >= a.compareTo(b)) to avoid having a compareTo return
something that isn't a number, but which has its own <= operator.
Never give a sucker an even break!

[floitsch, 2012/07/05 17:48:49]

Done.

   }
 
   static num max(num a, num b) {
-    // NaNs are handled correctly since the compareTo function always considers
-    // them to be bigger than any other operand.
-    return (a.compareTo(b) < 0) ? b : a;
+    if (a is num) {  // TODO(floitsch): move this into one if.
+      if (b is num) {
+        // Special case for NaN and -0.0. If one argument is NaN return NaN.
+        // In the context of [min] we have furthermore -0.0 < 0.0.

[sra1, 2012/07/04 19:28:23]

[max]

[floitsch, 2012/07/05 17:48:49]

Done.

+        if (a < b) return a;

[sra1, 2012/07/04 19:28:23]

max(1,2) ==> 1  ?

make sure the test covers  A x A where A includes {NaN, -oo, -1, -1.0, -0.0,
0.0, 1.0, 1, +oo}

[Lasse Reichstein Nielsen, 2012/07/05 09:34:41]

Yes, swap either the return or the comparison, but not both :)

[floitsch, 2012/07/05 17:48:49]

Done.

+        if (a == b) {
+          if (a is double && a == 0.0) {

[Lasse Reichstein Nielsen, 2012/07/05 09:34:41]

if (a == 0) return a + b;  // -0.0 if both are -0.0, otherwise 0.0 (or 0 if both
are int).

This does require that 0==0.0 and 0.0==-0.0 (and all the symmetric and
transitive consequences), but we should have that.

[floitsch, 2012/07/05 17:48:49]

done.

+            if (b is double) {
+              if (1.0 / a < 0.0) {
+                // a == -0.0, and b == -0.0 or 0.0.
+                return b;
+              }
+              // a == 0.0.
+              return a;
+            }
+            // a == -0.0 or 0.0, and b == 0.
+            return b;
+          }
+          return a;
+        }
+        if (a != a) return a;
+        return b;
+      }

[Lasse Reichstein Nielsen, 2012/07/05 09:34:41]

Here you know that 'a' is num and 'b' is not, so you know what a.compareTo does.
(What does it do? throw?)

[floitsch, 2012/07/05 17:48:49]

rewritten.

+    }
+    return a.compareTo(b) >= 0 ? a : b;
   }
 
   /**