Repository: netlify/gocommerce
Pull request #156 (2018-11-02 17:18:59)

Fix rounding inaccuracies with quantity
**- Summary**

There was a problem when using a quantity higher than 1 because
quantities were applied after rounding, therefore multiplying
rounding inaccuracies which is not acceptable.

Currently the amounts output in line items does not reflect the more
accurate values. There needs to be an API change to display single
and total amounts for items in orders.

**- Test plan**

Adjusted a test to take the now accurate amounts.

**- Description for the changelog**

Fix increase of rounding inaccuracies through item quantity


Patch:
diff --git a/calculator/calculator.go b/calculator/calculator.go
index 6fb26f8..4c3651c 100644
--- a/calculator/calculator.go
+++ b/calculator/calculator.go
@@ -173,6 +173,38 @@ func (t *Tax) AppliesTo(country, productType string) bool {
 	return applies
 }
 
+func calculateAmountsForSingleItem(settings *Settings, lineLogger logrus.FieldLogger, jwtClaims map[string]interface{}, params PriceParameters, item Item, multiplier uint64) ItemPrice {
+	itemPrice := ItemPrice{Quantity: item.GetQuantity()}
+
+	singlePrice := item.PriceInLowestUnit() * multiplier
+	_, itemPrice.Subtotal = calculateTaxes(singlePrice, item, params, settings)
+
+	// apply discount to original price
+	coupon := params.Coupon
+	if coupon != nil && coupon.ValidForType(item.ProductType()) && coupon.ValidForProduct(item.ProductSku()) {
+		itemPrice.Discount = calculateDiscount(singlePrice, coupon.PercentageDiscount(), coupon.FixedDiscount(params.Currency)*multiplier)
+	}
+	if settings != nil && settings.MemberDiscounts != nil {
+		for _, discount := range settings.MemberDiscounts {
+
+			if jwtClaims != nil && claims.HasClaims(jwtClaims, discount.Claims) && discount.ValidForType(item.ProductType()) && discount.ValidForProduct(item.ProductSku()) {
+				lineLogger = lineLogger.WithField("discount", discount.Claims)
+				itemPrice.Discount += calculateDiscount(singlePrice, discount.Percentage, discount.FixedDiscount(params.Currency)*multiplier)
+			}
+		}
+	}
+
+	discountedPrice := uint64(0)
+	if itemPrice.Discount < singlePrice {
+		discountedPrice = singlePrice - itemPrice.Discount
+	}
+
+	itemPrice.Taxes, itemPrice.NetTotal = calculateTaxes(discountedPrice, item, params, settings)
+	itemPrice.Total = int64(itemPrice.NetTotal + itemPrice.Taxes)
+
+	return itemPrice
+}
+
 // CalculatePrice will calculate the final total price. It takes into account
 // currency, country, coupons, and discounts.
 func CalculatePrice(settings *Settings, jwtClaims map[string]interface{}, params PriceParameters, log logrus.FieldLogger) Price {
@@ -193,34 +225,7 @@ func CalculatePrice(settings *Settings, jwtClaims map[string]interface{}, params
 			"product_sku":  item.ProductSku(),
 		})
 
-		itemPrice := ItemPrice{Quantity: item.GetQuantity()}
-
-		singlePrice := item.PriceInLowestUnit()
-		_, itemPrice.Subtotal = calculateTaxes(singlePrice, item, params, settings)
-
-		// apply discount to original price
-		coupon := params.Coupon
-		if coupon != nil && coupon.ValidForType(item.ProductType()) && coupon.ValidForProduct(item.ProductSku()) {
-			itemPrice.Discount = calculateDiscount(singlePrice, coupon.PercentageDiscount(), coupon.FixedDiscount(params.Currency))
-		}
-		if settings != nil && settings.MemberDiscounts != nil {
-			for _, discount := range settings.MemberDiscounts {
-
-				if jwtClaims != nil && claims.HasClaims(jwtClaims, discount.Claims) && discount.ValidForType(item.ProductType()) && discount.ValidForProduct(item.ProductSku()) {
-					lineLogger = lineLogger.WithField("discount", discount.Claims)
-					itemPrice.Discount += calculateDiscount(singlePrice, discount.Percentage, discount.FixedDiscount(params.Currency))
-				}
-			}
-		}
-
-		discountedPrice := uint64(0)
-		if itemPrice.Discount < singlePrice {
-			discountedPrice = singlePrice - itemPrice.Discount
-		}
-
-		itemPrice.Taxes, itemPrice.NetTotal = calculateTaxes(discountedPrice, item, params, settings)
-
-		itemPrice.Total = int64(itemPrice.NetTotal + itemPrice.Taxes)
+		itemPrice := calculateAmountsForSingleItem(settings, lineLogger, jwtClaims, params, item, 1)
 
 		lineLogger.WithFields(
 			logrus.Fields{
@@ -233,11 +238,13 @@ func CalculatePrice(settings *Settings, jwtClaims map[string]interface{}, params
 
 		price.Items = append(price.Items, itemPrice)
 
-		price.Subtotal += (itemPrice.Subtotal * itemPrice.Quantity)
-		price.Discount += (itemPrice.Discount * itemPrice.Quantity)
-		price.NetTotal += (itemPrice.NetTotal * itemPrice.Quantity)
-		price.Taxes += (itemPrice.Taxes * itemPrice.Quantity)
-		price.Total += (itemPrice.Total * int64(itemPrice.Quantity))
+		// avoid issues with rounding when multiplying by quantity before taxation
+		itemPriceMultiple := calculateAmountsForSingleItem(settings, lineLogger, jwtClaims, params, item, item.GetQuantity())
+		price.Subtotal += itemPriceMultiple.Subtotal
+		price.Discount += itemPriceMultiple.Discount
+		price.NetTotal += itemPriceMultiple.NetTotal
+		price.Taxes += itemPriceMultiple.Taxes
+		price.Total += itemPriceMultiple.Total
 	}
 
 	price.Total = int64(price.NetTotal + price.Taxes)
@@ -325,43 +332,6 @@ const (
 	fracMask = 1<<shift - 1
 )
 
-// Round returns the nearest integer, rounding half away from zero.
-//
-// Special cases are:
-//	Round(±0) = ±0
-//	Round(±Inf) = ±Inf
-//	Round(NaN) = NaN
-func Round(x float64) float64 {
-	// Round is a faster implementation of:
-	//
-	// func Round(x float64) float64 {
-	//   t := Trunc(x)
-	//   if Abs(x-t) >= 0.5 {
-	//     return t + Copysign(1, x)
-	//   }
-	//   return t
-	// }
-	bits := math.Float64bits(x)
-	e := uint(bits>>shift) & mask
-	if e < bias {
-		// Round abs(x) < 1 including denormals.
-		bits &= signMask // +-0
-		if e == bias-1 {
-			bits |= uvone // +-1
-		}
-	} else if e < bias+shift {
-		// Round any abs(x) >= 1 containing a fractional component [0,1).
-		//
-		// Numbers with larger exponents are returned unchanged since they
-		// must be either an integer, infinity, or NaN.
-		const half = 1 << (shift - 1)
-		e -= bias
-		bits += half >> e
-		bits &^= fracMask >> e
-	}
-	return math.Float64frombits(bits)
-}
-
 func rint(x float64) uint64 {
-	return uint64(Round(x))
+	return uint64(math.Round(x))
 }


Test patch:
diff --git a/calculator/calculator_test.go b/calculator/calculator_test.go
index ea9fb84..226a281 100644
--- a/calculator/calculator_test.go
+++ b/calculator/calculator_test.go
@@ -208,15 +208,11 @@ func TestCouponWithVATWhenPRiceIncludeTaxesWithQuantity(t *testing.T) {
 	params := PriceParameters{"USA", "USD", coupon, []Item{&TestItem{quantity: 2, price: 100, itemType: "test", vat: 9}}}
 	price := CalculatePrice(settings, nil, params, testLogger)
 
-	// todo: This result is wrong because a rounding inaccuracy is quantified
-	// Therefore the tax amount is not 9% of the net total
-	// Correct net total: 165
-	// Correct tax amount: 15
 	validatePrice(t, price, Price{
-		Subtotal: 184,
+		Subtotal: 183,
 		Discount: 20,
-		NetTotal: 166,
-		Taxes:    14,
+		NetTotal: 165,
+		Taxes:    15,
 		Total:    180,
 	})
 }