Skip to content

Conversation

@filiph
Copy link
Contributor

@filiph filiph commented Oct 22, 2019

I'm not sure this is a good idea, from educational perspective. But it does show that we have BigInt. And it shows attention to detail — Dart does think of big integers.

This will continue computing fib until the number fits into memory. Nobody will scroll that far.

The current version shows "seams" when you scroll far enough -- the int becomes JavaScript's num in the background, and switches to floating point when the int overflows, so you get numbers like 1.234234+e113.

In contrast, the new version shows:

Screen Shot 2019-10-22 at 11 31 53 AM

And then later:

Screen Shot 2019-10-22 at 11 32 04 AM

@filiph filiph requested a review from legalcodes October 22, 2019 19:05
@googlebot googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Oct 22, 2019
@sfshaza2
Copy link
Contributor

Not @legalcodes, but it LGTM

Copy link
Contributor

@legalcodes legalcodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a bunch Filip, FWIW I'm all for using BigInt in this context as you suggest.
Can you run compile.sh from src/_packages/dartpad_picker/ ?
Without doing so, the example still sees the old code.

https://github.com/flutter/website/blob/master/src/_packages/dartpad_picker/README.md

I've raised #3143 to add this info to the main README so it's a little easier to find!

@filiph
Copy link
Contributor Author

filiph commented Oct 29, 2019

Oh sorry, I missed compile.sh completely. I ran it and I think it also compiled the previous change to the flutterpad samples (#3139).

@filiph filiph requested a review from legalcodes October 29, 2019 20:26
@legalcodes legalcodes merged commit fbb79eb into master Oct 29, 2019
@filiph filiph deleted the fib-bigint branch October 29, 2019 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Contributor has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants