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

parseFloat not good for type conversion #266

Closed
brianc opened this issue Feb 11, 2013 · 10 comments
Closed

parseFloat not good for type conversion #266

brianc opened this issue Feb 11, 2013 · 10 comments

Comments

@brianc
Copy link
Owner

brianc commented Feb 11, 2013

The type conversion for floats should be reexamined. Making a note here until I have time to look @ it further. context:

https://gist.github.com/shtylman/4757910

https://twitter.com/defunctzombie/status/301085763188166656

@freewil
Copy link

freewil commented Feb 11, 2013

I think this is related #107

@freewil
Copy link

freewil commented Feb 11, 2013

Maybe it's just time to use a string for numeric types?

@defunctzombie
Copy link
Contributor

I suggested the use of num as another option since it will preserve value.

If not that, then I think string for numeric types are the way to go. The reason I would favor something like num would be to make it clearer that you can't just do val - 5 or whatnot and expect a correct result in js. If you return strings, you still allow the user to make such mistakes without being aware of the problems.

@freewil
Copy link

freewil commented Feb 12, 2013

I suggested the use of num as another option since it will preserve value.

I don't think this is a good solution. I agree with your concern over users doing dumb things, and is a good reason why this should be called out and documented clearly in the README. I don't think it's a good idea to lock users into any specific arbitrary precision library.

If not that, then I think string for numeric types are the way to go.

👍

@defunctzombie
Copy link
Contributor

@brianc any thoughts on making it just return a string? I have updated my code to use a custom parser for this that has been returning string and that has been better. I think string is the simplest lowest common thing we can do :)

@rpedela
Copy link
Contributor

rpedela commented Mar 29, 2013

What about wrapping it with a BigNumber class for bigint and numeric? The class simply stores a string version of the number. But it is easier to test if the object is a BigNumber such as:

if (num instanceof BigNumber) {
  // use your favorite big number arithmetic library
}

I did this for a custom JSON parser since we needed to support large numbers on our server. You can find the custom JSON parser here: https://github.com/datalanche/json-bignum

@brianc
Copy link
Owner Author

brianc commented Mar 29, 2013

That is an interesting and cool idea. You're saying basically wrap the string result of a number in a class that does nothing but hold the string so one could use 'instanceof' instead of just having to "know" a particular column is a number?

The only counter I have to that is you could easily make that an add-on module and snap it in to the type system instead of it being included with node-Postgres itself. Something like 'require("pg-big-number")'

Long term I am trying to make node-postgres less opinionated and push more features into other modules to be more "node-like"

Right now it suffers from being older than the idea of "do one tiny thing in each node module" itself.

On Mar 29, 2013, at 4:53 PM, Ryan Pedela [email protected] wrote:

What about wrapping it with a BigNumber class for int64 and numeric? The class simply stores a string version of the number. But it is easier to test if the object is a BigNumber such as:

if (num instanceof BigNumber) {
// use your favorite big number arithmetic library
}
I did this for a custom JSON parser since we needed to support large numbers on our server. You can find the custom JSON parser here: https://github.com/datalanche/json-bignum


Reply to this email directly or view it on GitHub.

@defunctzombie
Copy link
Contributor

@brianc I was talking about that earlier so that people would not try to
use the value as a raw string by accident.

On Fri, Mar 29, 2013 at 6:32 PM, Brian C [email protected] wrote:

That is an interesting and cool idea. You're saying basically wrap the
string result of a number in a class that does nothing but hold the string
so one could use 'instanceof' instead of just having to "know" a particular
column is a number?

The only counter I have to that is you could easily make that an add-on
module and snap it in to the type system instead of it being included with
node-Postgres itself. Something like 'require("pg-big-number")'

Long term I am trying to make node-postgres less opinionated and push more
features into other modules to be more "node-like"

Right now it suffers from being older than the idea of "do one tiny thing
in each node module" itself.

On Mar 29, 2013, at 4:53 PM, Ryan Pedela [email protected]
wrote:

What about wrapping it with a BigNumber class for int64 and numeric? The
class simply stores a string version of the number. But it is easier to
test if the object is a BigNumber such as:

if (num instanceof BigNumber) {
// use your favorite big number arithmetic library
}
I did this for a custom JSON parser since we needed to support large
numbers on our server. You can find the custom JSON parser here:
https://github.com/datalanche/json-bignum


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHubhttps://github.com//issues/266#issuecomment-15663790
.

@rpedela
Copy link
Contributor

rpedela commented Mar 29, 2013

@brianc Yes. I think a pg-bignum module would be fine too. We are currently switching our database from MySQL to Postgres. I could look into creating the module since we need to do it anyway.

@brianc
Copy link
Owner Author

brianc commented Apr 11, 2013

this is now fixed in pg @v1.0

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

4 participants