I think I have identified a good way to proceed.
A. I will first clean up the current code to prepare the LongBigInt
/ BigIntegerBigInt
split. In particular, channel direct constructor calls through BigInt.apply
, optimize a few of the current methods, port some of the Spire tests. This would be done preserving binary and source compatibility. I’d PR this against 2.13 so it could be merged ASAP.
B. Perform the breaking changes with minimal changes: make BigInt
abstract, introduce a final BigIntegerBigInt
concrete instance with private constructor.
C. Preserving source/binary compatibility with the previous step, add the LongBigInt
concrete instance, migrate code from Spire’s SafeLong.
Questions:
-
Is there a thing as “too much test code” for
BigInt
? I’ll to err on the cautious side due to amount of changes this introduces. -
What would be the roadblocks to have this merged? Does it introduce risk for too small a benefit? Do I need to demonstrate performance benefits to have the merge approved in 2.14?
-
I have one question about the design. At the final step,
BigInt
will have two implementations, one holding aLong
, one holding aBigInteger
. Currently, Spire’s implementation has the invariant that theBigInteger
instance must not hold an integer that’s a validLong
. That enables a few additional optimizations but forces all instance creations to go through a size test, and adds a bit of mental overhead when reading the code. What do you think? -
After step B), the modifications would be merged against 2.14. There is no branch named 2.14 yet, right?