Skip to content

Commit 0eae925

Browse files
rstamrstam
committed
CSHARP-2001: Decimal128.ToDecimal should not throw when when the conversion can succeed with a loss of precision.
1 parent 5fbf93f commit 0eae925

File tree

2 files changed

+112
-45
lines changed

2 files changed

+112
-45
lines changed

‎src/MongoDB.Bson/ObjectModel/Decimal128.cs

Lines changed: 59 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright 2016 MongoDB Inc.
1+
/* Copyright 2016-2017 MongoDB Inc.
22
*
33
* Licensed under the Apache License, Version 2.0 (the "License");
44
* you may not use this file except in compliance with the License.
@@ -37,7 +37,9 @@ public struct Decimal128 : IConvertible, IComparable<Decimal128>, IEquatable<Dec
3737
private const short __maxSignificandDigits = 34;
3838

3939
// private static fields
40-
private static readonly UInt128 __maxSignificand = UInt128.Parse("9999999999999999999999999999999999");
40+
private static readonly UInt128 __maxSignificand = UInt128.Parse("9999999999999999999999999999999999"); // must be initialized before Decimal128.Parse is called
41+
private static readonly Decimal128 __maxDecimalValue = Decimal128.Parse("79228162514264337593543950335");
42+
private static readonly Decimal128 __minDecimalValue = Decimal128.Parse("-79228162514264337593543950335");
4143
private static readonly Decimal128 __maxValue = Decimal128.Parse("9999999999999999999999999999999999E+6111");
4244
private static readonly Decimal128 __minValue = Decimal128.Parse("-9999999999999999999999999999999999E+6111");
4345

@@ -714,44 +716,62 @@ public static decimal ToDecimal(Decimal128 d)
714716
{
715717
if (Flags.IsFirstForm(d._highBits))
716718
{
717-
var exponent = Decimal128.GetExponent(d);
718-
719-
// try to get the exponent within the range of 0 to -28
720-
if (exponent > 0)
719+
if (Decimal128.IsZero(d))
721720
{
722-
d = Decimal128.DecreaseExponent(d, 0);
723-
exponent = Decimal128.GetExponent(d);
721+
return decimal.Zero;
724722
}
725-
else if (exponent < -28)
723+
else if (Decimal128.Compare(d, __minDecimalValue) < 0 || Decimal128.Compare(d, __maxDecimalValue) > 0)
726724
{
727-
d = Decimal128.IncreaseExponent(d, -28);
728-
exponent = Decimal128.GetExponent(d);
725+
throw new OverflowException("Value is too large or too small to be converted to a Decimal.");
729726
}
730727

731-
// try to get the significand to have zeros for the high order 32 bits
728+
var isNegative = Decimal128.IsNegative(d);
729+
var exponent = Decimal128.GetExponent(d);
732730
var significand = Decimal128.GetSignificand(d);
731+
732+
// decimal significand must fit in 96 bits
733733
while ((significand.High >> 32) != 0)
734734
{
735-
uint remainder;
736-
var significandDividedBy10 = UInt128.Divide(significand, (uint)10, out remainder);
737-
if (remainder != 0)
735+
uint remainder; // ignored
736+
significand = UInt128.Divide(significand, 10, out remainder);
737+
exponent += 1;
738+
}
739+
740+
// decimal exponents must be between 0 and -28
741+
if (exponent > 0)
742+
{
743+
// bring exponent within range
744+
while (exponent > 0)
738745
{
739-
break;
746+
significand = UInt128.Multiply(significand, (uint)10);
747+
exponent -= 1;
740748
}
741-
exponent += 1;
742-
significand = significandDividedBy10;
743749
}
750+
else if (exponent < -28)
751+
{
752+
// check if exponent is too far out of range to possibly be brought within range
753+
if (exponent < -56)
754+
{
755+
return decimal.Zero;
756+
}
744757

758+
// bring exponent within range
759+
while (exponent < -28)
760+
{
761+
uint remainder; // ignored
762+
significand = UInt128.Divide(significand, (uint)10, out remainder);
763+
exponent += 1;
764+
}
745765

746-
if (exponent < -28 || exponent > 0 || (significand.High >> 32) != 0)
747-
{
748-
throw new OverflowException("Value is too large or too small to be converted to a Decimal.");
766+
if (significand.Equals(UInt128.Zero))
767+
{
768+
return decimal.Zero;
769+
}
749770
}
750771

751772
var lo = (int)significand.Low;
752773
var mid = (int)(significand.Low >> 32);
753774
var hi = (int)significand.High;
754-
var isNegative = Decimal128.IsNegative(d);
755775
var scale = (byte)(-exponent);
756776

757777
return new decimal(lo, mid, hi, isNegative, scale);
@@ -1193,28 +1213,24 @@ private static string ClampOrRound(ref int exponent, string significandString)
11931213
return significandString;
11941214
}
11951215

1196-
private static Decimal128 DecreaseExponent(Decimal128 x, short goal)
1216+
private static void TryDecreaseExponent(ref UInt128 significand, ref short exponent, short goal)
11971217
{
1198-
if (Decimal128.IsZero(x))
1218+
if (significand.Equals(UInt128.Zero))
11991219
{
1200-
// return a zero with the desired exponent
1201-
return Decimal128.FromComponents(Decimal128.IsNegative(x), goal, UInt128.Zero);
1220+
exponent = goal;
1221+
return;
12021222
}
12031223

1204-
var exponent = GetExponent(x);
1205-
var significand = GetSignificand(x);
12061224
while (exponent > goal)
12071225
{
12081226
var significandTimes10 = UInt128.Multiply(significand, (uint)10);
1209-
if (significandTimes10.CompareTo(Decimal128.__maxSignificand) <= 0)
1227+
if (significandTimes10.CompareTo(Decimal128.__maxSignificand) > 0)
12101228
{
12111229
break;
12121230
}
12131231
exponent -= 1;
12141232
significand = significandTimes10;
12151233
}
1216-
1217-
return Decimal128.FromComponents(Decimal128.IsNegative(x), exponent, significand);
12181234
}
12191235

12201236
private static Decimal128 FromComponents(bool isNegative, short exponent, UInt128 significand)
@@ -1243,16 +1259,14 @@ private static UInt128 GetSignificand(Decimal128 d)
12431259
return new UInt128(GetSignificandHighBits(d), GetSignificandLowBits(d));
12441260
}
12451261

1246-
private static Decimal128 IncreaseExponent(Decimal128 x, short goal)
1262+
private static void TryIncreaseExponent(ref UInt128 significand, ref short exponent, short goal)
12471263
{
1248-
if (Decimal128.IsZero(x))
1264+
if (significand.Equals(UInt128.Zero))
12491265
{
1250-
// return a zero with the desired exponent
1251-
return Decimal128.FromComponents(Decimal128.IsNegative(x), goal, UInt128.Zero);
1266+
exponent = goal;
1267+
return;
12521268
}
12531269

1254-
var exponent = GetExponent(x);
1255-
var significand = GetSignificand(x);
12561270
while (exponent < goal)
12571271
{
12581272
uint remainder;
@@ -1264,8 +1278,6 @@ private static Decimal128 IncreaseExponent(Decimal128 x, short goal)
12641278
exponent += 1;
12651279
significand = significandDividedBy10;
12661280
}
1267-
1268-
return Decimal128.FromComponents(Decimal128.IsNegative(x), exponent, significand);
12691281
}
12701282

12711283
private static short MapDecimal128BiasedExponentToExponent(short biasedExponent)
@@ -1883,27 +1895,30 @@ private int CompareNegativeNumbers(Decimal128 x, Decimal128 y)
18831895
private int ComparePositiveNumbers(Decimal128 x, Decimal128 y)
18841896
{
18851897
var xExponent = GetExponent(x);
1898+
var xSignificand = GetSignificand(x);
18861899
var yExponent = GetExponent(y);
1900+
var ySignificand = GetSignificand(y);
1901+
18871902
var exponentDifference = Math.Abs(xExponent - yExponent);
18881903
if (exponentDifference <= 66)
18891904
{
18901905
// we may or may not be able to make the exponents equal but we won't know until we try
18911906
// but we do know we can't eliminate an exponent difference larger than 66
18921907
if (xExponent < yExponent)
18931908
{
1894-
x = IncreaseExponent(x, yExponent);
1895-
y = DecreaseExponent(y, xExponent);
1909+
TryIncreaseExponent(ref xSignificand, ref xExponent, yExponent);
1910+
TryDecreaseExponent(ref ySignificand, ref yExponent, xExponent);
18961911
}
18971912
else if (xExponent > yExponent)
18981913
{
1899-
x = DecreaseExponent(x, yExponent);
1900-
y = IncreaseExponent(y, xExponent);
1914+
TryDecreaseExponent(ref xSignificand, ref xExponent, yExponent);
1915+
TryIncreaseExponent(ref ySignificand, ref yExponent, xExponent);
19011916
}
19021917
}
19031918

19041919
if (xExponent == yExponent)
19051920
{
1906-
return GetSignificand(x).CompareTo(GetSignificand(y));
1921+
return xSignificand.CompareTo(ySignificand);
19071922
}
19081923
else
19091924
{

‎tests/MongoDB.Bson.Tests/ObjectModel/Decimal128Tests.cs

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright 2016 MongoDB Inc.
1+
/* Copyright 2016-2017 MongoDB Inc.
22
*
33
* Licensed under the Apache License, Version 2.0 (the "License");
44
* you may not use this file except in compliance with the License.
@@ -59,6 +59,58 @@ public void Decimal(string valueString, string s)
5959
result.Should().Be(value);
6060
}
6161

62+
[Theory]
63+
[InlineData("0", "0")]
64+
[InlineData("79228162514264337593543950335", "79228162514264337593543950335")]
65+
[InlineData("1E1", "10")]
66+
[InlineData("1E2", "100")]
67+
[InlineData("1E28", "10000000000000000000000000000")]
68+
[InlineData("1E-28", "0.0000000000000000000000000001")]
69+
[InlineData("1E-29", "0")]
70+
[InlineData("10E-29", "0.0000000000000000000000000001")]
71+
[InlineData("10E-30", "0")]
72+
[InlineData("100E-30", "0.0000000000000000000000000001")]
73+
[InlineData("100E-31", "0")]
74+
[InlineData("1E-55", "0")]
75+
[InlineData("1E-56", "0")]
76+
[InlineData("1E-57", "0")]
77+
[InlineData("1E-99", "0")]
78+
[InlineData("1E-6111", "0")]
79+
[InlineData("10000.0000000000000000000000001", "10000")] // see: CSHARP-2001
80+
public void ToDecimal_should_return_expected_result(string valueString, string expectedResultString)
81+
{
82+
var subject = Decimal128.Parse(valueString);
83+
var expectedResult = decimal.Parse(expectedResultString);
84+
85+
var result = Decimal128.ToDecimal(subject);
86+
87+
result.Should().Be(expectedResult);
88+
}
89+
90+
[Theory]
91+
[InlineData("1E29")]
92+
[InlineData("79228162514264337593543950336")]
93+
[InlineData("79228162514264337593543950340")]
94+
[InlineData("792281625142643375935439503351E-1")]
95+
[InlineData("7922816251426433759354395033511E-2")]
96+
[InlineData("9999999999999999999999999999999999")]
97+
[InlineData("9999999999999999999999999999999999E+6111")]
98+
[InlineData("-79228162514264337593543950336")]
99+
[InlineData("-79228162514264337593543950340")]
100+
[InlineData("-9999999999999999999999999999999999")]
101+
[InlineData("-9999999999999999999999999999999999E+6111")]
102+
[InlineData("Infinity")]
103+
[InlineData("-Infinity")]
104+
[InlineData("NaN")]
105+
public void ToDecimal_should_throw_when_value_is_out_of_range(string valueString)
106+
{
107+
var subject = Decimal128.Parse(valueString);
108+
109+
var exception = Record.Exception(() => Decimal128.ToDecimal(subject));
110+
111+
exception.Should().BeOfType<OverflowException>();
112+
}
113+
62114
[Theory]
63115
[InlineData((byte)0, "0")]
64116
[InlineData((byte)1, "1")]

0 commit comments

Comments
 (0)