gentooboontoo / js-quantities

JavaScript library for quantity calculation and unit conversion
http://gentooboontoo.github.io/js-quantities/
MIT License
396 stars 102 forks source link

Quantities with 'NaN' scalar #20

Closed glepretre closed 10 years ago

glepretre commented 10 years ago

I noticed an unexpected behavior when trying to instantiate quantities directly from user input.

I know the correct way to instantiate a negative quantity is, for example:

var negativeQty = new Qty('-1 m');

This returns a perfectly valid qty:

{
  scalar: -1,
  baseScalar: -1,
  signature: 1,
  _conversionCache: {},
  numerator: [
    "<meter>"
  ],
  denominator: [
    "<1>"
  ],
  initValue": "-1 m",
  _isBase": true
}

But, these strings return qty objects with NaN scalar:

new Qty('- 1 m'); // .toString() --> "NaN *1*1*m"
new Qty('+ 1 m'); // .toString() --> "NaN *1*1*m"
new Qty('- m');   // .toString() --> "NaN m"
new Qty('+ m');   // .toString() --> "NaN m"

This is the object returned by "- 1 m":

{
  scalar: NaN,
  baseScalar: NaN,
  signature: 1,
  _conversionCache: {},
  numerator: [
    "<1>",
    "<meter>"
  ],
  denominator: [
    "<1>"
  ],
  initValue: "- 1 m",
  _isBase: true
}

Use case: Users with no scientific or programming background are more likely to write "- 1 m" as input.

This is easy to handle in our client apps but the question is: should Qty return quantities with NaN scalar? I would rather expect it to return a valid qty or an Error.

rage-shadowman commented 10 years ago

It sounds to me like throwing an error would be good in this case.

gentooboontoo commented 10 years ago

You are right. This is a bug. No quantity should be returned with NaN as scalar. "- 1 m" should return a correct quantity because whitespaces are not intended to be significant. "- m" and "+ m" should raise an 'Error'. I don't currently see useful cases for them unless proven otherwise.

gentooboontoo commented 10 years ago

Fixed by commits d74a28ddccc7757938e33cb2d3be47b49f97a93a and d4673067e296f4b1e49e55f5db872fc77e61faff.