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

[BigInt tests] ✅🐰 Codable #259

Open
wants to merge 2 commits into
base: biginteger
Choose a base branch
from

Conversation

LiarPrincess
Copy link

Please read the #242 Using tests from “Violet - Python VM written in Swift” before.


🐰 Discussion

Currently Codable uses description which is a String with radix 10.

Following options may be better:

  • String with radix 32
    • ✅ human readable - just like current format; not sure if we care about this property
    • ✅ much shorter in transport - though it may compress a bit worse
    • ✅ faster to encode/decode - though the current implementation is slow
  • Binary - just an array of UInt32 with sign
    • ❌ not human readable
    • ✅ ultra compact
    • ✅ ultra fast to encode/decode

We need to remember that BigInt can get really big. In [BigInt tests] 💀 Init from float 754 we operate on integers that have 3000 digits with 32 radix. With radix 10 they would be much longer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant