Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double.toRadixString not implemented #2671

Closed
terrylucas opened this issue Apr 20, 2012 · 5 comments
Closed

Double.toRadixString not implemented #2671

terrylucas opened this issue Apr 20, 2012 · 5 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. closed-duplicate Closed in favor of an existing report P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@terrylucas
Copy link
Contributor

Even though my number is an int the VM some how thinks it's a double. Certainly implementing toRadixString will fix it but I'm wondering if there's a bigger problem that r1 is not an int but a double?

class Bug {
  static String convertToHexString(int r) {
    int r1 = Bug._changeTintShadeColor(r, .10);
    String rHex = Bug._numAs2DigitHex(r1);

    // TODO(terry) 15.toRadixString(16) return 'F' on Dartium not f as in JS.
    // bug: <http://code.google.com/p/dart/issues/detail?id=2670>
    return "$rHex".toLowerCase();
  }

  static String _numAs2DigitHex(int v) {
    String hex = "${v.toRadixString(16)}";
    if (hex.length == 1) {
      hex = "0${hex}";
    }
    return hex;
  }

  static num _clamp(num value, num min, num max) =>
      Math.max(Math.min(max, value), min);

  static int _changeTintShadeColor(num v, num delta) =>
      Bug._clamp((v + (255 * delta)).round(), 0, 255);
}

Bug.convertToHexString(10);

@terrylucas
Copy link
Contributor Author

Looks like changing _numAs2DigitHex's line

    String hex = "${v.toRadixString(16)}";
to
    String hex = "${v.toInt().toRadixString(16)}";

Works around the problem. I've downgraded the bug.


Removed Priority-Critical label.
Added Priority-High label.

@madsager
Copy link
Contributor

cc @floitschG.
Added Triaged label.

@DartBot
Copy link

DartBot commented Apr 23, 2012

This comment was originally written by @mhausner


Actually, everything works as intended here. r1 is a double because the delta parameter to _changeTintShadeColor is a double, so the expression v + (255 * .1) becomes double. round() and min() and max() will return double values.

the double value is assigned to int r1, which in non-checked mode is not a problem. If you run this code in checked mode, it would produce a type check error.

I just ran the code and realize that there is a problem even before the assignment to r1. The method toRadixString() is not implemented for double values, so you get an exception. You do need to convert the value to integer first, which is what your "workaround" does.

In any case, this is not a bug. I'll let Florian close the bug if he agrees with my assessment.

@DartBot
Copy link

DartBot commented Apr 23, 2012

This comment was originally written by @mhausner


I guess reading the summary would have helped and I wouldn't have had to "discover" the same toRadixString() issue. Nevertheless, the "bigger problem" you hint at is not a problem, as per my explanation.

@floitschG
Copy link
Contributor

Agreed.


Added Duplicate label.
Marked as being merged into #1896.

@terrylucas terrylucas added Type-Defect P1 A high priority bug; for example, a single project is unusable or has many test failures area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. closed-duplicate Closed in favor of an existing report labels Apr 25, 2012
dart-bot pushed a commit that referenced this issue Oct 14, 2020
git log --oneline cf9795f3bb209504c349e20501f0b4b8ae31530c..f0c7771b38155d3829a60d60b5dba2784b100811
f0c7771b Set first version with null safety to 2.12 (#2684)
df1140af Warn from get, when mixed mode (#2590)
765778c0 Simplify test detection (#2682)
afd66ea2 Inline the single test asset. (#2681)
059e4796 Simplify the logic for unicode and colors in output (#2679)
35ddaec2 Dartify test tool (#2680)
62f26401 Example for User-Agent (#2678)
e8b4b114 fixes: #2670 pub global activate commands always exit cmd on windows. (#2671)
93e703b1 Improve stack traces in many tests (#2673)
5b540a39 Fix experiments tests for Dart 2.11 (#2672)
b0ac77d8 Bump dependency on pkg:analyzer (#2662)
73f0906e Removed @alwaysThrows (#2642)
88e0a83c Fixed bug in adding dependency to empty dependencies key (#2640)
135d9fa0 Pub add/remove now remove dependencies key if it makes them empty (#2639)
f4cc9673 Fix "pubpsec" typo (#2641)
4686d74d Adding an existing package is now a dataError (#2638)
1e93f47c Checks pubspec keys for potential typos (#2616)
fa5f51ef Vendor yaml_edit (#2633)
ac6d307f Adding a `pub remove` command (#2620)
9d236e00 Adding the `pub add` command (#2618)
04e237f7 Drop upper bound instead of using "any" while resolving in "pub outdated" (#2623)
93954f33 Use InternetAddress.tryParse over try/catch (#2626)
638c81c9 Refine publishing message (#2624)


Allow github


Embed pub as a library into dartdev

Change-Id: Iadc6acb5c3425dfb8848db89820e6c9c8caf16ba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/167574
Reviewed-by: Jonas Jensen <jonasfj@google.com>
Commit-Queue: Sigurd Meldgaard <sigurdm@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. closed-duplicate Closed in favor of an existing report P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

4 participants