Why do you use NativeDigits instead of '0' ?

[schlebe]

In GetSignificantDigits() function, I can find following line

			var valueString = value.ToString().TrimEnd(CultureInfo.CurrentCulture.NumberFormat.NativeDigits[0][0]);

Why do you use NativeDigits instead of '0' ?

		var valueString = value.ToString().TrimEnd('0');

[AdamWhiteHat]

In case someone's configured language or writing system uses something other than Arabic numerals.

[schlebe]

Yes it is interesting for displaying something ... not for computing !

Personally, I found that render code more complex and less performant.

I encounter same problem when working in C# (or VB.Net or C++) when my CultureInfo is fr-BE ! (french in Belgium)

To solve this problem in code that has nothing to do with display (as it is in BigDecimal, except for ToString()), I set temporarily CultureInfo to en-US.

It is as MVC (Model View Controler principe) ! BigDecimal is principaly a model code. All that is computation in it must be done without using NativeDigits() function that is View code

GetSignificantDigits() is pure computational code and using NativeDigits() in it is an programming MVC fault !

The big problem in BigDecimal code is that ToString() is used for pure computational code !

I propose to replace CultureInfo.CurrentCulture.NumberFormat.NativeDigits[0][0] by char('0').

What do you thing about ?

Using explicitely InvariantCulture as explain on StackOverflow is certainly a better solution !

https://stackoverflow.com/questions/9760237/what-does-cultureinfo-invariantculture-mean
https://stackoverflow.com/questions/9760237/what-does-cultureinfo-invariantculture-mean/14418644#14418644

In GetSignificantDigits() function the solution would be

var valueString = value.ToString(CultureInfo.InvariantCulture).TrimEnd('0');

I post this message because I have modified all your test's code so that these tests work when fr-BE culture is used !

[AdamWhiteHat]

Okay, I see. Yeah, you are right; I should be using InvariantCulture here and the '0' character. It doesn't matter what CurrentCulture the user has set, we are not displaying strings to the user, the user will never see this string, what is important here is internal consistency, and if we use InvariantCulture, we know for certain what character will be used for zero and Microsoft gives assurances that InvariantCulture will not change.
This is an easy fix. I will probably fix this today.

I should probably re-visit all the places I call .ToString() in the library and evaluate its usage to ensure I am not making the same error.

The big problem in BigDecimal code is that ToString() is used for pure computational code !

I have no problem replacing BigIntegerHelper.GetSignificantDigits with an all-numerical approach, but I suspect that performance will actually go down because of it. Here is why:
Consider the difference between calling .ToString().Length on a BigInteger to get the number of digits long the number is versus using the extension method BigIntegerHelper.GetLength(this BigInteger source) which uses arithmetic to accomplish the same thing.
For each digit in the BigInteger, ToString probably involves mapping a single digit, 0-9 to its character equivalent via a lookup table.
Whereas BigIntegerHelper.GetLength will count the number of times it can divide the input by 10 before reaching zero.
The number of steps is the same (the number of digits in the number), but on one hand we have a look-up table, and on the other we have division. So in theory, the purely numerical route should be slower.

I went ahead and implemented the pure numerical version of GetSignificantDigits, and created two tests calling each version multiple time and wrapped them in a stopwatch to test this theory.
And... Its really hard to tell. By my testing, the method is taking on average about 3/100ths of a millisecond to execute.
I used the NonParallelizable attribute on the tests to force them to run in series. I used the Repeat attribute to run each test 40 times back to back per test. I discarded the first measurement because of the large startup/warmup time that dwarfs the subsequent run times and made this measurement an outlier. I tried running the tests in different order as well as separately, and got consistent timings each time. Taking the arithmetic average of 40 measurements each, I get the following averages:
0.022097ms - ToString method
0.029881ms - All-numerical method

So it appears that the all-numerical method takes 35.22% more time, but then again we are talking measurements on the average of 3/100th of a millisecond here so its hard to trust such a result and the time taken is so insignificant its probably not worthy of our concern.

Now if these timing measurements don't comport at all with your experience, one possible reason is I made all of the variables that GetSignificantDigits was called on, as well as the expected value to be compared against, I made them either a const or static because I recognized that the memory allocation sections of the code may take variable amounts of time and dominate the runtimes.

Anyways, all this is to say I will probably keep the ToString approach to things, but I will definitely take your suggestion of using InvariantCulture and '0'. I will probably create an issue out of this discussion and then check in a fix for it here shortly.