Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decimal.sum to support no arguments #207

Open
yassh opened this issue Sep 2, 2022 · 7 comments
Open

Decimal.sum to support no arguments #207

yassh opened this issue Sep 2, 2022 · 7 comments

Comments

@yassh
Copy link

yassh commented Sep 2, 2022

Currently, an error occurs when zero arguments is passed to Decimal.sum.

const decimals = []
Decimal.sum(...decimals) // Error occurs

Lodash's _.sum and Ramda's R.sum returns 0 when passed an array with zero elements.

const numbers = []
_.sum(numbers) // => 0
R.sum(numbers) // => 0

Similarly, it would be nice if Decimal.sum returned new Decimal(0) when called with zero arguments.

const decimals = []
Decimal.sum(...decimals) // => new Decimal(0)
@MikeMcl
Copy link
Owner

MikeMcl commented Sep 2, 2022

I think this has been requested before.

Should Decimal.max and min then behave similarly?

And the prototype methods?

new Decimal(1).add()    // => new Decimal(1)

Personally, I think it is better (safer) to throw when undefined is passed.

@yassh
Copy link
Author

yassh commented Sep 3, 2022

Should Decimal.max and min then behave similarly?

Math.max returns -Infinity and Math.min returns Infinity when no arguments are passed.

Math.max() // => -Infinity
Math.min() // => Infinity

So, I propose that Decimal.max and Decimal.min behave similarly.

Decimal.max() // => new Decimal(-Infinity)
Decimal.min() // => new Decimal(Infinity)

@gterras
Copy link

gterras commented Oct 10, 2022

Just my user voice but I highly appreciate the simplicity of the lib as it is. Not having to remember these kind of subtleties is a plus.

@michaelhays
Copy link

For my use case, I need to retrieve the sum of a highly nested and filtered array of objects, and it would be very convenient if Decimal.sum() == 0.

I'd love to be able to do this:

const value = Decimal.sum(...myArray.filter(filterFunc).map(mapFunc))

But because myArray.filter(filterFunc).map(mapFunc) might be an empty array, I'm currently having to do this:

const value =
  myArray.filter(filterFunc).length > 0
    ? Decimal.sum(...myArray.filter(filterFunc).map(mapFunc))
    : new Decimal(0)

(In reality, I have many more filters and maps than this example, but it paints the picture)

Though honestly, I think it would be ideal if Decimal.sum accepted a single array as an argument (like lodash.sumBy), where Decimal.sum([]) == 0.

@gterras
Copy link

gterras commented Oct 10, 2022

The concern is legit since some popular libs do this, it could make a useful option.

Note that you can still one line your sum with

myArr.reduce((a: Decimal, b: Decimal) => a.plus(b), new Decimal(0)) 
// or even without acc
myArr.reduce((a: Decimal, b: Decimal) => a.plus(b)) 

@hunterwilhelm
Copy link

hunterwilhelm commented Nov 21, 2022

I just ran into this issue, it is not obvious that Decimal.sum(...[]) will throw an error.
I am putting Decimal.sum(0, ...myArray) now to fix this issue and it's unappealing.

@michaelhays
Copy link

Yeah, still don't really love the API here.

I ended up just making a utility function which accepts an array (instead of multiple arguments), and returns 0 for an empty array or undefined argument:

import Decimal from 'decimal.js'

/**
 * Sum an array of Decimal values, returning 0 for undefined or empty arrays.
 */
export default function sumDecimal(decimals?: Decimal[]): Decimal {
  return !decimals || decimals.length === 0
    ? new Decimal(0)
    : Decimal.sum(...decimals)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants