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

Move from floating point to decimal #508

Open
xoudini opened this issue Jul 25, 2018 · 2 comments
Open

Move from floating point to decimal #508

xoudini opened this issue Jul 25, 2018 · 2 comments

Comments

@xoudini
Copy link

xoudini commented Jul 25, 2018

Double-precision floating point values are currently used in three locations, although a more exact decimal representation would be better suited for the purpose. Additionally, the front-end doesn't strip any insignificant decimals, leaving floating point inaccuracies visible to the user, as can be seen here:

fp

Suggested fix:

Change all instances of DataTypes.DOUBLE to DataTypes.DECIMAL, and migrate the respective columns in the database. A precision of two decimals should suffice, so e.g. DataTypes.DECIMAL(3, 2) would probably be enough for all intents and purposes.

For further information, see sequelize docs on data types, as well as PostgreSQL docs on numeric types.

Affected files:

@qzuw
Copy link
Contributor

qzuw commented Sep 4, 2018

Just a note, @Chamion did something to partially address this, but this bug is still around at least on listing page.

@ollikehy
Copy link
Contributor

ollikehy commented Apr 10, 2019

The floating point error still exist, atleast in the teacher view sum column. A possible fix to make the frontend look decent is to force the value into fixed two decimals with for example:
toFixed(2).replace(/[.,]00$/, "")
The regex is so that whole numbers are left as they are.

Related Table Cell

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

3 participants