MikeMcl / bignumber.js

A JavaScript library for arbitrary-precision decimal and non-decimal arithmetic
http://mikemcl.github.io/bignumber.js
MIT License
6.64k stars 741 forks source link

Store alphabet internally as object (fixes #309 and #250) #310

Open jbrower2 opened 2 years ago

jbrower2 commented 2 years ago

This change does not modify the external contract of the library.

This change replaces the internal ALPHABET string with an object of the form:

{
  // the original alphabet
  original: '0123456789abcdef',
  length: 16,

  // whether this alphabet is compatible with decimal (starts with '0123456789')
  decimalCompatible: true,

  // delegates directly to original.charAt
  charAt(i) { ... },

  // case-insensitive version of indexOf, that takes in the base as an optional param
  charIndex(c, b) { ... }
}

I've tested this locally and everything seems to function as I expect. I also modified the existing unit tests for the BigNumber constructor to use mixed-case strings.

MikeMcl commented 2 years ago

Your implementation is quite neat - I like the way the BigNumber constructor function is simplified with the removal of two variables etc. - but:

const log = x => { console.log(x.toString()) };

// Default ALPHABET.
BigNumber.set({ ALPHABET: '0123456789abcdefghijklmnopqrstuvwxyz' });

log(new BigNumber('ff', 16));    // '255'
log(new BigNumber('FF', 16));    // '255'
log(new BigNumber('Ff', 16));    // '255'

BigNumber.set({ ALPHABET: '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ$_' });

log(new BigNumber('ff', 16));    // '255'
log(new BigNumber('FF', 16));    // 'NaN' !?
log(new BigNumber('Ff', 16));    // 'NaN'  !?

Is this intended? Surely not. The hexadecimal alphabet is the same in both ALPHABETs above, so the values logged should be the same for each.

To be honest, I am not sure it is worth you spending any more time on this. I am happy with the existing behaviour, and have documented it now and also fixed #250.