Merge pull request #268 from typelevel/wip-money-precision

Fix precision issues with Money

project/plugins.sbt

@@ -6,7 +6,7 @@ addSbtPlugin("org.scala-native" % "sbt-scalajs-crossproject" % "0.1.0")
 
 addSbtPlugin("org.scala-native" % "sbt-scala-native" % "0.2.1")
 
-addSbtPlugin("org.scalariform" % "sbt-scalariform" % "1.6.0")
+addSbtPlugin("org.scalariform" % "sbt-scalariform" % "1.5.1")
 
 addSbtPlugin("com.typesafe.sbt" % "sbt-osgi" % "0.8.0")
 

shared/src/main/scala/squants/market/CurrencyExchangeRate.scala

@@ -43,6 +43,16 @@ case class CurrencyExchangeRate(base: Money, counter: Money) extends Ratio[Money
   /** convert  */
   def *(money: Money) = convert(money)
 
+
+  /**
+    * Override methods from Ratio to ensure BigDecimal precision math is applied
+    *
+    * @param m Money
+    * @return
+    */
+  override def convertToBase(m: Money): Money = base * (m / counter)
+  override def convertToCounter(m: Money): Money = counter * (m / base)
+
   /**
    * Returns the rate formatted in as standard FX Quote"
    * @return

shared/src/main/scala/squants/market/Money.scala

@@ -56,8 +56,15 @@ final class Money private (val amount: BigDecimal)(val currency: Currency)
    *
    * @return String
    */
-  override def toString: String = amount.toString + " " + currency.code
+  override def toString: String = amount.underlying.stripTrailingZeros.toString + " " + currency.code
 
+  /**
+    * Converts the amount to the given currency and returns a string formatted with the original precision and the currency code
+    *
+    * @param c Currency
+    * @param context MoneyContext required for conversion
+    * @return
+    */
   def toString(c: Currency)(implicit context: MoneyContext): String = in(c).toString
 
   /**
@@ -127,7 +134,17 @@ final class Money private (val amount: BigDecimal)(val currency: Currency)
    * @param that BigDecimal
    * @return Money
    */
-  def *(that: BigDecimal): Money = new Money(amount * that)(currency)
+  def times(that: BigDecimal): Money = new Money(amount * that)(currency)
+  def *(that: BigDecimal): Money = times(that)
+
+  /**
+    * Overrides Quantity.times to ensure BigDecimal math is performed
+    *
+    * @param that Double
+    * @return Quantity
+    */
+  override def times(that: Double): Money = new Money(amount * that)(currency)
+  override def *(that: Double): Money = times(that)
 
   /**
    * Multiplies this money by that [[squants.market.CurrencyExchangeRate]] and returns the equal value in the other currency.
@@ -145,7 +162,17 @@ final class Money private (val amount: BigDecimal)(val currency: Currency)
    * @param that BigDecimal
    * @return Money
    */
-  def /(that: BigDecimal): Money = new Money(amount / that)(currency)
+  def divide(that: BigDecimal): Money = new Money(amount / that)(currency)
+  def /(that: BigDecimal): Money = divide(that)
+
+  /**
+    * Overrides Quantity.divide to ensure BigDecimal math is performed
+    *
+    * @param that Double
+    * @return Quantity
+    */
+  override def divide(that: Double): Money = new Money(amount / that)(currency)
+  override def /(that: Double): Money = divide(that)
 
   /**
    * Integer divides this money by that BigDecimal and returns the remainder

shared/src/test/scala/squants/QuantityChecks.scala

@@ -18,8 +18,7 @@ import org.scalacheck.Gen
 trait QuantityChecks {
 
   type TestData = Int
-  val posNum = Gen.posNum[TestData]
+  val posNum: Gen[TestData] = Gen.posNum[TestData]
   val tol = 1e-13
   implicit val tolTime = Seconds(tol)
-
 }

shared/src/test/scala/squants/market/MarketChecks.scala

@@ -20,23 +20,45 @@ import org.scalacheck.Prop._
 object MarketChecks extends Properties("Market") with QuantityChecks {
 
   property("Money + Money = Money") = forAll(posNum, posNum) { (a: TestData, b: TestData) ⇒
-    USD(a + b) == USD(a.toDouble) + USD(b.toDouble)
+    USD(a + b) == USD(a) + USD(b)
   }
 
   property("Money - Money = Money") = forAll(posNum, posNum) { (a: TestData, b: TestData) ⇒
-    USD(a - b) == USD(a.toDouble) - USD(b.toDouble)
+    USD(a - b) == USD(a) - USD(b)
   }
 
   property("Money * Double = Money") = forAll(posNum, posNum) { (a: TestData, b: TestData) ⇒
-    USD(a * b) == USD(a.toDouble) * b.toDouble
+    USD(a * b) == USD(a) * b
   }
 
   property("Money / Double = Money") = forAll(posNum, posNum) { (a: TestData, b: TestData) ⇒
-    USD(a.toDouble / b) == USD(a.toDouble) / b.toDouble
+    USD(BigDecimal(a) / b) == USD(a) / b
   }
 
   property("Money = ExchangeRate * Money") = forAll(posNum, posNum) { (a: TestData, b: TestData) ⇒
-    USD(b.toDouble / a.toDouble) == CurrencyExchangeRate(USD(1), JPY(a.toDouble)) * JPY(b.toDouble) &&
-      USD(b.toDouble / a.toDouble) == JPY(b.toDouble) * CurrencyExchangeRate(USD(1), JPY(a.toDouble))
+    USD(BigDecimal(b) / a) == CurrencyExchangeRate(USD(1), JPY(a)) * JPY(b) &&
+      USD(BigDecimal(b) / a) == JPY(b) * CurrencyExchangeRate(USD(1), JPY(a))
+  }
+
+  property("(variations of) Money * Double + Money * Double = (Money * Double) * 2") = forAll(posNum, posNum) { (a: TestData, b: TestData) ⇒
+    val m = USD(a.toDouble)
+    val x = b.toDouble
+
+    x * m + x * m == ((m * x) * 2) &&
+      m * x + x * m == ((m * x) * 2) &&
+      m * x + m * x == ((m * x) * 2) &&
+      x * m + m * x == ((m * x) * 2) &&
+      x * m + x * m == ((x * m) * 2) &&
+      m * x + x * m == ((x * m) * 2) &&
+      m * x + m * x == ((x * m) * 2) &&
+      x * m + m * x == ((x * m) * 2)
+  }
+
+  property("Money / Double + Money / Double = (Money / Double) * 2") = forAll(posNum, posNum) { (a: TestData, b: TestData) ⇒
+    implicit val tolUSD = USD(1e-32)
+    val m = if (a > 0) USD(a.toDouble) else USD(1)
+    val x = if (b > 0) b.toDouble else 1d
+
+    (m / x + m / x) ~= ((m / x) * 2)
   }
 }

shared/src/test/scala/squants/market/MoneySpec.scala

@@ -142,7 +142,7 @@ class MoneySpec extends FlatSpec with Matchers {
     x != y should be(right = true)
   }
 
-  it should "return proper result on max/min operation with an implicit MoneyContext in scope" in {
+  it should "return a proper result on max/min operation with an implicit MoneyContext in scope" in {
     val r1 = CurrencyExchangeRate(USD(1), JPY(100))
     val r2 = CurrencyExchangeRate(USD(1), EUR(.75))
     implicit val moneyContext = MoneyContext(USD, defaultCurrencySet, List(r1, r2))
@@ -152,17 +152,39 @@ class MoneySpec extends FlatSpec with Matchers {
     y.moneyMin(x) should be(y)
   }
 
-  it should "return proper result when adding like currencies with no MoneyContext in scope" in {
+  it should "return a proper result when adding like currencies with no MoneyContext in scope" in {
     USD(1) + USD(2) should be(USD(3))
     USD(1).plus(USD(2)) should be(USD(3))
   }
 
-  it should "return proper result when subtracting like currencies with no MoneyContext in scope" in {
+  it should "return a proper result when subtracting like currencies with no MoneyContext in scope" in {
     USD(5) - USD(2) should be(USD(3))
     USD(5).minus(USD(2)) should be(USD(3))
   }
 
-  it should "return proper result when dividing like currencies with no MoneyContext in scope" in {
+  it should "return a proper result with no additional precision loss when multiplying by a primitive numeric" in {
+    val x: Double = 4992
+    val m: Money = EUR(9709.6)
+
+    x * m + x * m should be((m * x) * 2)
+    m * x + x * m should be((m * x) * 2)
+    m * x + m * x should be((m * x) * 2)
+    x * m + m * x should be((m * x) * 2)
+
+    x * m + x * m should be((x * m) * 2)
+    m * x + x * m should be((x * m) * 2)
+    m * x + m * x should be((x * m) * 2)
+    x * m + m * x should be((x * m) * 2)
+  }
+
+  it should "return a proper result with no additional precision loss when dividing by a primitive number" in {
+    val d: Double = 4992d
+    val m: Money = EUR(9709.6)
+
+    m / d + m / d should be((m / d) * 2)
+  }
+
+  it should "return a proper result when dividing like currencies with no MoneyContext in scope" in {
     USD(10) / USD(2) should be(5)
     USD(10).divide(USD(2)) should be(5)
   }