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

#2379 - Fix computer multiplication in combine decimal places #2419

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

dheepak-aot
Copy link
Collaborator

@dheepak-aot dheepak-aot commented Oct 16, 2023

Floating point arithmetic issue in programming languages

image

https://stackoverflow.com/questions/45221535/javascript-floating-point-multiply-by-100-still-has-errors

Issue and fix

Certain decimal numbers when multiplied by 100 may not return precise mathematical result as we expect

image

Math.round() seems to be one of the solutions to handle this issue.

But in the combineDecimalPlaces method we are dividing and then multiplying by 100 again, which re-creates the same issue which was fixed by Math.round()

image

Hence: Divide and multiply of the rounded value by multiplier is removed.

File output

image

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 17.26% ( 2218 / 12852 )
Methods: 7.93% ( 129 / 1626 )
Lines: 20.01% ( 1945 / 9720 )
Branches: 9.56% ( 144 / 1506 )

@github-actions
Copy link

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 46.3% ( 300 / 648 )
Methods: 40% ( 32 / 80 )
Lines: 50.6% ( 251 / 496 )
Branches: 23.61% ( 17 / 72 )

@github-actions
Copy link

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 75.72% ( 555 / 733 )
Methods: 69.32% ( 61 / 88 )
Lines: 77.64% ( 486 / 626 )
Branches: 42.11% ( 8 / 19 )

Copy link
Collaborator

@andrewsignori-aot andrewsignori-aot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍
Thanks for the nice chat 😉

Copy link
Contributor

@ann-aot ann-aot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dheepak-aot dheepak-aot merged commit 2409b80 into main Oct 17, 2023
@dheepak-aot dheepak-aot temporarily deployed to DEV October 17, 2023 00:17 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV October 17, 2023 00:18 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV October 17, 2023 00:18 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV October 17, 2023 00:18 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV October 17, 2023 00:18 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV October 17, 2023 00:21 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot temporarily deployed to DEV October 17, 2023 00:21 — with GitHub Actions Inactive
@dheepak-aot dheepak-aot deleted the fix/#2379-ier-fix-computer-multiplication branch October 19, 2023 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants