Add support to define custom units #112
Conversation
|
I'm not sure it's clear that I don't have write access, so somebody will have to merge this if it's okay. |
| - `denominator` is an array containing the set of units that can be specified as a denominator for this unit | ||
|
|
||
| `unitDefinition` for prefixes is sligly different. | ||
| - `aliases` is an array of aliases for the unit, e.g. `["m","meter","meters","metre","metres"]` |
There was a problem hiding this comment.
I don't think "meter" is a good example for a prefix.
| ``` | ||
|
|
||
| Defining new units should be done carefully and tested thoroughly as it can | ||
| introduce conflicts while parsing other units. |
There was a problem hiding this comment.
Why not throw an error if you find a collision?
| `unitDefinition` is an array of definition values. For units it is: | ||
| - `aliases` is an array of aliases for the unit, e.g. `["m","meter","meters","metre","metres"]` | ||
| - `conversion` is a decimal number that can be multiplied to the number to | ||
| convert to the base unit | ||
| - `kind` is the kind of the unit. If you specify an unknown kind here, be | ||
| sure to include a unit with `isBase` set to true for doing conversions | ||
| - `numerator` is an array containing the set of units that can be specified as a numerator for this unit | ||
| - `denominator` is an array containing the set of units that can be specified as a denominator for this unit |
There was a problem hiding this comment.
Why an array? Why not build the array yourself internally out of an object that's passed in? Less chance of accidentally swapping order that way.
| if (isBase) { | ||
| if (BASE_UNITS.indexOf(unitDef) === -1) { | ||
| BASE_UNITS.push(unitDef); | ||
| } |
There was a problem hiding this comment.
else throw error? silent failures are difficult to debug.
PS - What happens if oldDef was a base unit? Does this leave the new one in the expected state?
| @@ -1476,6 +1476,27 @@ describe("js-quantities", function() { | |||
| }); | |||
| }); | |||
|
|
|||
| describe("Qty.defineUnit", function() { | |||
There was a problem hiding this comment.
unit/alias collisions throwing errors, and tests for that would be nice
maybe also a way to remove an existing unit
Introduce a new static method to provide runtime addition of units and prefixes. Conflicts are created at the user's peril, but it provides a path to add custom units without forking the repo and modifying source code.
fixes #96