Bug cleanup: unify on allowing radices from 2 to 36 in

Bug cleanup: unify on allowing radices from 2 to 36 in
num.toRadixString. Add test and documentation.

R=lrn@google.com,floitsch@google.com
BUG=dartbug.com/461

Committed: https://code.google.com/p/dart/source/detail?r=12562

[Lasse Reichstein Nielsen, 2012-09-19 11:43:54]

LGTM

https://chromiumcodereview.appspot.com/10937011/diff/1/lib/core/num.dart
File lib/core/num.dart (right):

https://chromiumcodereview.appspot.com/10937011/diff/1/lib/core/num.dart#newc...
lib/core/num.dart:45: * Converts the [num] to an [int] using [toInt] and then
converts to
Could we make this comment start with a single-line summary (which will be shown
in apidoc overviews).
The current sentence is a little too long for that.
Perhaps move the part about converting using [toInt] to a subsequent sentence.

https://chromiumcodereview.appspot.com/10937011/diff/1/lib/core/num.dart#newc...
lib/core/num.dart:47: * supported.
Either say that the radix must be in the supported range (the "must" implictly
saying that we'll throw an IllegalArgumentException or similar if it doesn't
satisfy this requirement) or write explictly that (and what) we throw if it's
not in the supported range.

Write that we use lower-case letters for digits above '9'.
(Since you test that they are lower-case, it should be documented).

https://chromiumcodereview.appspot.com/10937011/diff/1/lib/core/num.dart#newc...
lib/core/num.dart:48: */
Ok for now. I'm considering whether it really belongs on int instead of num.

https://chromiumcodereview.appspot.com/10937011/diff/1/tests/corelib/integer_...
File tests/corelib/integer_to_radix_string_test.dart (right):

https://chromiumcodereview.appspot.com/10937011/diff/1/tests/corelib/integer_...
tests/corelib/integer_to_radix_string_test.dart:23: } catch (e) {
Add "on IllegalArgumentException"?

[Mads Ager (google), 2012-09-19 14:16:13]

https://chromiumcodereview.appspot.com/10937011/diff/1/lib/core/num.dart
File lib/core/num.dart (right):

https://chromiumcodereview.appspot.com/10937011/diff/1/lib/core/num.dart#newc...
lib/core/num.dart:45: * Converts the [num] to an [int] using [toInt] and then
converts to
On 2012/09/19 11:43:54, Lasse Reichstein Nielsen wrote:
> Could we make this comment start with a single-line summary (which will be
shown
> in apidoc overviews).
> The current sentence is a little too long for that.
> Perhaps move the part about converting using [toInt] to a subsequent sentence.

Yes, changed to have a one-liner followed by a longer explanation.

https://chromiumcodereview.appspot.com/10937011/diff/1/lib/core/num.dart#newc...
lib/core/num.dart:47: * supported.
On 2012/09/19 11:43:54, Lasse Reichstein Nielsen wrote:
> Either say that the radix must be in the supported range (the "must" implictly
> saying that we'll throw an IllegalArgumentException or similar if it doesn't
> satisfy this requirement) or write explictly that (and what) we throw if it's
> not in the supported range.
> 
> Write that we use lower-case letters for digits above '9'.
> (Since you test that they are lower-case, it should be documented).

Done.

https://chromiumcodereview.appspot.com/10937011/diff/1/lib/core/num.dart#newc...
lib/core/num.dart:48: */
On 2012/09/19 11:43:54, Lasse Reichstein Nielsen wrote:
> Ok for now. I'm considering whether it really belongs on int instead of num.

Yes, I agree. I think we should use it to int in a follow-up change.

https://chromiumcodereview.appspot.com/10937011/diff/1/tests/corelib/integer_...
File tests/corelib/integer_to_radix_string_test.dart (right):

https://chromiumcodereview.appspot.com/10937011/diff/1/tests/corelib/integer_...
tests/corelib/integer_to_radix_string_test.dart:23: } catch (e) {
On 2012/09/19 11:43:54, Lasse Reichstein Nielsen wrote:
> Add "on IllegalArgumentException"?

Yes! Thanks, that caught another inconsistency. dart2js ended up with a JS
exception and the VM threw a string. Unified on IllegalArgumentException.