Don't throw exception for non-integer args to fixnum int32,int64 op ==

pkg/fixnum/lib/src/int32.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of fixnum;
 
 /**
  * An immutable 32-bit signed integer, in the range [-2^31, 2^31 - 1].
  * Arithmetic operations may overflow in order to maintain this range.
  */
 class int32 implements intx {

[justinfagnani, 2013/07/26 23:44:40]

btw, I should have mentioned this last review, but I think these classes should
start with a capital "I".

[Chris Bracken, 2013/07/27 01:07:11]

Agreed - will create a separate CL for this to avoid diff noise in this one.

 
   /**
    * The maximum positive value attainable by an [int32], namely
    * 2147483647.
    */
   static const int32 MAX_VALUE = const int32._internal(0x7FFFFFFF);
 
   /**
    * The minimum positive value attainable by an [int32], namely
    * -2147483648.
@@ skipping 109 matching lines @@
   final int _i;
 
   const int32._internal(int i) : _i = i;
 
   /**
    * Constructs an [int32] from an [int].  Only the low 32 bits of the input
    * are used.
    */
   int32.fromInt(int i) : _i = (i & 0x7fffffff) - (i & 0x80000000);
 
-  // Convert an [int] or [intx] to an [int32].  Note that an [int64]
-  // will be truncated.
-  int _convert(other) {
-    if (other == null) {
-      throw new ArgumentError(null);
-    } else if (other is intx) {
-      return other.toInt32()._i;
-    } else if (other is int) {
-      return other;
-    } else {
-      throw new Exception("Can't retrieve 32-bit int from $other");
+  // Returns the [int] representation of the specified value or null for
+  // incompatible types.
+  int _toInt(val) {

[justinfagnani, 2013/07/26 23:44:40]

all uses of this exception the one in ==() throw an argument error. how about
putting the throw here and just using custom logic in ==()?

[Chris Bracken, 2013/07/27 01:07:11]

Done.

+    if (val is int32) {
+      return val._i;
+    } else if (val is int) {
+      return val;
     }
+    return null;
   }
 
   // The +, -, * , &, |, and ^ operaters deal with types as follows:
   //
   // int32 + int => int32
   // int32 + int32 => int32
   // int32 + int64 => int64
   //
   // The %, ~/ and remainder operators return an int32 even with an int64
   // argument, since the result cannot be greater than the value on the
   // left-hand side:
   //
   // int32 % int => int32
   // int32 % int32 => int32
   // int32 % int64 => int32
 
   intx operator +(other) {
     if (other is int64) {
       return this.toInt64() + other;

[justinfagnani, 2013/07/26 23:44:40]

for all these "this.toInt64()" calls, can you instead delegate to the RHS to
avoid creating a new object? Here you could have:

if (other is int64) {
  return other + this;
}

[Chris Bracken, 2013/07/27 01:07:11]

Done.

[Chris Bracken, 2013/07/27 01:07:11]

As the code currently stands, it won't help. int64._promote() will do the same
thing. For the comparison, bit operators and addition I think we can make
changes to int64 that would take advantage of this. Will create a separate CL
for these.

     }
-    return new int32.fromInt(_i + _convert(other));
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return new int32.fromInt(_i + intVal);
+    }
+    throw new ArgumentError(other);
   }
 
   intx operator -(other) {
     if (other is int64) {
       return this.toInt64() - other;
     }
-    return new int32.fromInt(_i - _convert(other));
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return new int32.fromInt(_i - intVal);
+    }
+    throw new ArgumentError(other);
   }
 
   int32 operator -() => new int32.fromInt(-_i);
 
   intx operator *(other) {
     if (other is int64) {
       return this.toInt64() * other;
     }
     // TODO(rice) - optimize
     return (this.toInt64() * other).toInt32();
   }
 
   int32 operator %(other) {
     if (other is int64) {
       // Result will be int32
       return (this.toInt64() % other).toInt32();
     }
-    return new int32.fromInt(_i % _convert(other));
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return new int32.fromInt(_i % intVal);
+    }
+    throw new ArgumentError(other);
   }
 
   int32 operator ~/(other) {
     if (other is int64) {
-      // Result will be int32
       return (this.toInt64() ~/ other).toInt32();
     }
-    return new int32.fromInt(_i ~/ _convert(other));
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return new int32.fromInt(_i ~/ intVal);
+    }
+    throw new ArgumentError(other);
   }
 
   int32 remainder(other) {
     if (other is int64) {
-      // Result will be int32
       int64 t = this.toInt64();
       return (t - (t ~/ other) * other).toInt32();
     }
     return this - (this ~/ other) * other;
   }
 
   int32 operator &(other) {
     if (other is int64) {
       return (this.toInt64() & other).toInt32();
     }
-    return new int32.fromInt(_i & _convert(other));
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return new int32.fromInt(_i & intVal);
+    }
+    throw new ArgumentError(other);
   }
 
   int32 operator |(other) {
     if (other is int64) {
       return (this.toInt64() | other).toInt32();
     }
-    return new int32.fromInt(_i | _convert(other));
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return new int32.fromInt(_i | intVal);
+    }
+    throw new ArgumentError(other);
   }
 
   int32 operator ^(other) {
     if (other is int64) {
       return (this.toInt64() ^ other).toInt32();
     }
-    return new int32.fromInt(_i ^ _convert(other));
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return new int32.fromInt(_i ^ intVal);
+    }
+    throw new ArgumentError(other);
   }
 
   int32 operator ~() => new int32.fromInt(~_i);
 
   int32 operator <<(int n) {
     if (n < 0) {
       throw new ArgumentError("$n");
     }
     n &= 31;
     return new int32.fromInt(_i << n);
@@ skipping 25 matching lines @@
       value = (_i >> n) & ((1 << (32 - n)) - 1);
     }
     return new int32.fromInt(value);
   }
 
   /**
    * Returns [true] if this [int32] has the same numeric value as the
    * given object.  The argument may be an [int] or an [intx].
    */
   bool operator ==(other) {
-    if (other == null) {
-      return false;
-    }
     if (other is int64) {
       return this.toInt64() == other;
     }
-    return _i == _convert(other);
+    return _i == _toInt(other);
   }
 
   int compareTo(Comparable other) {
     if (other is int64) {
       return this.toInt64().compareTo(other);
     }
-    return _i.compareTo(_convert(other));
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return _i.compareTo(intVal);
+    }
+    throw new ArgumentError(other);
   }
 
   bool operator <(other) {
     if (other is int64) {
       return this.toInt64() < other;
     }
-    return _i < _convert(other);
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return _i < intVal;
+    }
+    throw new ArgumentError(other);
+
   }
 
   bool operator <=(other) {
     if (other is int64) {
       return this.toInt64() < other;
     }
-    return _i <= _convert(other);
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return _i <= intVal;
+    }
+    throw new ArgumentError(other);
   }
 
   bool operator >(other) {
     if (other is int64) {
       return this.toInt64() < other;
     }
-    return _i > _convert(other);
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return _i > intVal;
+    }
+    throw new ArgumentError(other);
+
   }
 
   bool operator >=(other) {
     if (other is int64) {
       return this.toInt64() < other;
     }
-    return _i >= _convert(other);
+    var intVal = _toInt(other);
+    if (intVal != null) {
+      return _i >= intVal;
+    }
+    throw new ArgumentError(other);
   }
 
   bool get isEven => (_i & 0x1) == 0;
   bool get isMaxValue => _i == 2147483647;
   bool get isMinValue => _i == -2147483648;
   bool get isNegative => _i < 0;
   bool get isOdd => (_i & 0x1) == 1;
   bool get isZero => _i == 0;
 
   int get hashCode => _i;
@@ skipping 13 matching lines @@
   }
 
   int toInt() => _i;
   int32 toInt32() => this;
   int64 toInt64() => new int64.fromInt(_i);
 
   String toString() => _i.toString();
   String toHexString() => _i.toRadixString(16);
   String toRadixString(int radix) => _i.toRadixString(radix);
 }