Add Round function #66
Conversation
|
Hey, @GadQ I have made the suggested changes. cc: @rayepps |
There was a problem hiding this comment.
Overall, looks good. I appreciate the work @shan-shaji 🎉 A few comments.
package-lock.json
Outdated
| @@ -0,0 +1,7634 @@ | |||
| { | |||
There was a problem hiding this comment.
Radash uses yarn (and yarn.lock) to track dependencies so we don't want to commit this package-lock.json file.
src/number.ts
Outdated
| @@ -13,3 +13,10 @@ export const toInt = (value: any, defaultValue: number = 0) => { | |||
| const result = parseInt(value) | |||
| return isNaN(result) ? defaultValue : result | |||
| } | |||
|
|
|||
| export const round = (value: number, precision?: number): number => { | |||
There was a problem hiding this comment.
I think we should default the precision arg to something common. If I call round(2.239) and I get back 2.239 that is probably unexpected and unhelpful in most cases. Defaulting this to a precision of 2 might make sense?
I could be wrong about this, what do you think?
There was a problem hiding this comment.
That's nice @rayepps i will work on it.
src/tests/number.test.ts
Outdated
| test('rounds 20.456 with precision 2', () => { | ||
| const result = _.round(20.456, 2); | ||
| assert.strictEqual(result, 20.46); | ||
| }) |
There was a problem hiding this comment.
I'd love to see a few more test cases here that cover common rounding at different precisions. Just a few
0.005at precision1?19.9999at precision4?2.5at precision9
It might seem a bit dull but when it comes to rounding it could impact finance/cost/price so better very very safe than sorry.
|
Hi @shan-shaji, would you like to continue work on this PR over at radashi-org/radashi, a new actively maintained fork of Radash? Please let me know! 😄 |
|
Yeah sure @aleclarson i can work on it. |
Closes #61