Skip to content

Commit

Permalink
Fix #3: Qty.toPrec returns wrong result with some precision
Browse files Browse the repository at this point in the history
Bug reported by Adam Abrams
  • Loading branch information
Julien Sanchez committed Oct 1, 2013
1 parent b41406c commit 5c67458
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 12 deletions.
7 changes: 5 additions & 2 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,12 @@ rounding issues.

### Rounding

Qty#toPrec(precision) with precision as number, string or quantity standing for the minimum significative quantity.
Returns a new quantity.
Qty#toPrec(precision) : returns the nearest multiple of quantity passed as precision

var qty = new Qty('5.17 ft');
qty.toPrec('ft'); // => 5 ft
qty.toPrec('0.5 ft'); // => 5 ft
qty.toPrec('0.25 ft'); // => 5.25 ft
qty.toPrec('0.1 ft'); // => 5.2 ft
qty.toPrec('0.05 ft'); // => 5.15 ft
qty.toPrec('0.01 ft'); // => 5.17 ft
Expand All @@ -134,6 +134,9 @@ Returns a new quantity.
qty.toPrec('10 m'); // => 10 m
qty.toPrec(0.1); // => 6.3 m

var qty = new Qty('1.146 MPa');
qty.toPrec('0.1 bar'); // => 1.15 MPa

### Text output

// if target_units string passed, the unit will first be converted to target_units before output.
Expand Down
11 changes: 7 additions & 4 deletions spec/quantitiesSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,7 @@ describe('js-quantities', function() {
expect(qty.toPrec(new Qty('0.05 ft')).toString()).toBe("5.15 ft");
expect(qty.toPrec(new Qty('0.01 ft')).toString()).toBe("5.17 ft");
expect(qty.toPrec(new Qty('0.0001 ft')).toString()).toBe("5.17 ft");
expect(qty.toPrec(new Qty('0.25 ft')).toString()).toBe("5.25 ft");
});

it('should allow string as precision parameter', function() {
Expand All @@ -976,11 +977,13 @@ describe('js-quantities', function() {
expect(qty.toPrec(new Qty('0.01 MPa')).toString()).toBe("1.15 MPa");
expect(qty.toPrec(new Qty('dbar')).toString()).toBe("1.15 MPa");

// Tests below are mainly a safety net because not sure if there is
// any usefulness to do things like that
qty = new Qty('5.171234568 ft');
expect(qty.toPrec(new Qty('m')).toString()).toBe("7 ft");
expect(qty.toPrec(new Qty('dm')).toString()).toBe("5.2 ft");
expect(qty.toPrec(new Qty('cm')).toString()).toBe("5.18 ft");
expect(qty.toPrec(new Qty('mm')).toString()).toBe("5.171 ft");
expect(qty.toPrec(new Qty('m')).toString()).toBe("6.561679790026246 ft");
expect(qty.toPrec(new Qty('dm')).toString()).toBe("5.249343832020998 ft");
expect(qty.toPrec(new Qty('cm')).toString()).toBe("5.183727034120736 ft");
expect(qty.toPrec(new Qty('mm')).toString()).toBe("5.170603674540682 ft");
});
});

Expand Down
25 changes: 19 additions & 6 deletions src/quantities.js
Original file line number Diff line number Diff line change
Expand Up @@ -722,12 +722,28 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
// return a new quantity with same units rounded
// at the nearest value according to quantity passed
// as precision
/**
* Returns the nearest multiple of quantity passed as
* precision
*
* @param {(Qty|string|number)} prec-quantity - Quantity, string formated
* quantity or number as expected precision
*
* @returns {Qty} Nearest multiple of prec_quantity
*
* @example
* new Qty('5.5 ft').toPrec('2 ft'); // returns 6 ft
* new Qty('0.8 cu').toPrec('0.25 cu'); // returns 0.75 cu
* new Qty('6.3782 m').toPrec('cm'); // returns 6.38 m
* new Qty('1.146 MPa').toPrec('0.1 bar'); // returns 1.15 MPa
*
*/
toPrec: function(prec_quantity) {
if(prec_quantity && prec_quantity.constructor === String) {
prec_quantity = new Qty(prec_quantity);
}
if(typeof prec_quantity === "number") {
prec_quantity = new Qty(prec_quantity+' '+this.units());
prec_quantity = new Qty(prec_quantity + ' ' + this.units());
}

if(!this.isUnitless()) {
Expand All @@ -739,11 +755,8 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI

if(prec_quantity.scalar === 0)
throw "Divide by zero";
var prec_rounded_result = Math.round(this.scalar/prec_quantity.scalar)*prec_quantity.scalar;

// Remove potential floating error based on prec_quantity exponent
var prec_exponent = exponent_of(prec_quantity.scalar);
prec_rounded_result = round(prec_rounded_result, -prec_exponent);
var prec_rounded_result = mul_safe(Math.round(this.scalar/prec_quantity.scalar),
prec_quantity.scalar);

return new Qty(prec_rounded_result + this.units());
},
Expand Down

0 comments on commit 5c67458

Please sign in to comment.