Calculating the total value of an invoice with precision

Question

I have written some functions to calculate the total cost of an invoice, gives its line items. Could someone please tell me if there seems to be a problem with the code? Since it is something as serious as handling people's money, I need to be extra careful. I used Big.js to make sure that I do not get any of the JS weirdness when it comes to arithmetical operations, but I am still unsure if I am doing it right.

Please take into account that the values in unitPrice in cents, and tax rates as integers. That means that a price of 12ドル.34 would be saved as 1234, and a tax of 19% would be saved as 19.

const calculateItemUnitTotal = (item) => {
 const unitPrice = new Big(item.unitPrice || 0);
 const taxRate = new Big(item.taxRate || 0).div(100);
 const total = unitPrice.times(taxRate.plus(1));
 const final = total.round(0, Big.roundHalfUp).toNumber(); // Round to nearest integer (cents)
 return final;
};
const calculateItemTotal = (item) => {
 const quantity = new Big(item.quantity || 0);
 const unitTotal = new Big(calculateItemUnitTotal(item));
 const total = quantity.times(unitTotal);
 const final = total.round(0, Big.roundHalfUp).toNumber(); // Round to nearest integer (cents)
 return final;
};
const calculateItemSubTotal = (item) => {
 const quantity = new Big(item.quantity || 0);
 const unitPrice = new Big(item.unitPrice || 0);
 const subTotal = quantity.times(unitPrice);
 return subTotal.round(0, Big.roundHalfUp).toNumber(); // Round to nearest integer (cents)
};
const calculateInvoiceSubtotal = (lineItems) => {
 if (!Array.isArray(lineItems)) return 0;
 const total = lineItems?.reduce((acc, item) => {
 const itemTotal = new Big(calculateItemSubTotal(item));
 return acc.plus(itemTotal);
 }, new Big(0));
 return total.round(0, Big.roundHalfUp).toNumber(); // Round to nearest integer (cents)
};
const calculateInvoiceTotal = (lineItems) => {
 if (!Array.isArray(lineItems)) return 0;
 const total = lineItems?.reduce((acc, item) => {
 const itemTotal = new Big(calculateItemTotal(item));
 return acc.plus(itemTotal);
 }, new Big(0));
 const final = total.round(0, Big.roundHalfUp).toNumber(); // Round to nearest integer (cents)
 return final;

• Neil Meyer: Why are the constants declared in the limited scope of a function?
• Enrique Moreno Tent: I try to make as many deterministic functions without side effects as much as possible.
• RoToRa: I'm not sure if this is reviewable. This is just a collection of functions without explanation. Code reviews usually require runnable code. BTW, I'm not sure if "Big.js" is appropriate. You are handling exclusively integers and "Big.js" is a decimal library. JavaScript has a BigInt class built in.
• radarbob: JS docs say: ...by calling the BigInt() function (without the new operator)
• radarbob: What is a "subTotal"? When does summing numbers evolve to subTotaling numbers? I don't see the purpose of making a distinction.

Answer 1

unit tests

The whole point of the OP code is to compute different results than FP would, a subtle difference that is challenging to see when just eyeballing some source code. This would have been stronger submission if accompanied by an automated test suite. As written, you are asking "did I follow a Big invocation pattern?" rather than "did I get the result correct down to the penny?"

specification

The OP code does not tell us the magnitudes it is expected to correctly operate upon, e.g. should this get the U.S. national debt correct to the penny?

In the Review Context you helpfully explain that only integer tax rates shall be supported, e.g. 19%. But assuming that "tax" describes what real world tax collection agencies levy, this is a rather surprising constraint, one worth commenting on so we understand where it comes from. A more usual requirement would be "tax rate shall be a rational number, with a power of ten in the denominator", ruling out e.g. a troublesome two-thirds of a percent rate.

contract

In calculateInvoiceSubtotal and calculateInvoiceTotal, caller has a clear responsibility to pass in an invoice datastructure. If they don't, it would be better to throw a fatal Exception than to erroneously claim it was an empty invoice worth zero dollars. Then folks will notice the buggy caller and fix the code, rather than silently accepting incorrect results.

consistently use same rounding mode

 const final = total.round(0, Big.roundHalfUp)...
 const final = total.round(0, Big.roundHalfUp)...
 return subTotal.round(0, Big.roundHalfUp)...
 return total.round(0, Big.roundHalfUp)...
 const final = total.round(0, Big.roundHalfUp)...

The documentation explains that

... the rounding mode used to round the results of these methods is determined by the value of the ... RM propert[y] of the Big number constructor.

Big.RM = Big.roundHalfUp

Even if you chose not to set the RM, all those copy-pasta code fragments plus comment would belong in a roundToCents() helper.

• Enrique Moreno Tent: I didn't consider that in some places taxes could be fractional. I guess this changes everything...
• J_H: No, it's easy. If city + county sales tax sums to eight and a quarter percent, then the rational number you're interested in is 825 / 10000, which plays nicely with Big.

Answer 2

Frameworks

Frameworks should reduce source code and reduce code complexity, at the same time reduce your workload.

In this case Big.js seams to be making your source code longer and the execution much slower.

The javascript number can represent signed integers from Number.MIN_SAFE_INTEGER -9007199254740991 to Number.MAX_SAFE_INTEGER 9007199254740991.

That range is more than enough to invoice the human race in cents for many millennia to come.

If you have a special need to round 0.5 up it's a very simple task to do that using the JavaScript Math API.

Destructuring assignment

Use destructuring assignment to extract properties from an object passed to a function.

Default parameters

Use default default parameters to set defaults for missing function parameters.

Nullish coalescing operator

Use nullish coalescing operator ?? to set a defaults for missing values

Spread syntax

Use spread syntax to pass arrays of parameter to functions. This avoids the need to test if array exists.

To call a function with an existing array use the spread operator in the call

Example

const calcTotal = (...items) => items.reduce((total, cost) => total + cost, 0);
const items = [1, 2, 3, 4];
calcTotal(...items); // spread the array

Naming

If you are naming many things with the same prefix it a very good sign indicating that these names are related.

Place the related names into an object to help organize, maintain and keep the source code readable.

In your code each function was prefixed with the word calculate in the example calculate is the name of the object that contains all the functions.

Note: that the object calculate has been frozen Object.freeze. This makes all the objects properties read only

Rewrite

The rewrite removes the dependency on Big.js and uses the standard JS Math API.

A function roundCents has been created to round up on the half point.

We avoid using single use variables and calculate values as needed. This results in all the function being simple one liners. Your ~40 lines of code (ignoring linking to big.js) becomes under 20 lines.

This halfs your workload and more than likely more than halfs the CPU and RAM overheads

const roundCents = cents = (cents % 0.5) >= 0.5 ? Math.ceil(cents) : Math.floor(cents);
const calculate = Object.freeze({
 unitTotal({unitPrice: price = 0, taxRate = 0}) {
 return roundCents(price + price * taxRate * 0.01); 
 },
 itemTotal(item) { 
 return roundCents((item.quantity ?? 0) * calculate.unitTotal(item)); 
 },
 itemSubTotal({quantity = 0, unitPrice: price = 0}) { 
 return roundCents(quantity * price); 
 },
 invoiceSubtotal(...items) { 
 return roundCents(items.reduce((total, item) => total + calculate.itemSubTotal(item), 0)); 
 },
 invoiceTotal(...items) { 
 return roundCents(items.reduce((total, item) => total + calculate.itemTotal(item), 0));
 },
});