Unit.toString causes side effects

[AlexYegupov]

Seems like after just logging to console my unit variable is converted into SI.

Here is my interactive commands:

% node                                                                      
> const math = require('mathjs')
undefined
> q0 = math.eval('1cm + 1 km') ; console.log(123)
123
undefined
> q0.toNumber()
100001
> q0.toNumber()
100001
> q0
Unit {
  units: [ { unit: [Object], prefix: [Object], power: 1 } ],
  dimensions: [ 0, 1, 0, 0, 0, 0, 0, 0, 0 ],
  value: 1000.01,
  fixPrefix: false,
  isUnitListSimplified: true }
> q0.toNumber()
1.00001
> q0.toNumber()
1.00001

Seems behavior: after just logging to console q0 is converted from cm to km.
Expected behavior: q0.toNumber() must still return 100001 because it wasn't converted

System info:

% lsb_release -a                                                 
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 16.04.3 LTS
Release:        16.04
Codename:       xenial

% node -v                                                       
v8.9.1

> math.version
'3.18.0'

[ericman314]

Thanks for reporting. Yes, there is a side effect when q0.toString is called. When outputting a unit, the prefix is chosen to make the output value somewhat of a reasonable magnitude. Changing the prefix for outputting does not alter the value of the unit itself, which is stored in q0.value and q0.dimensions. And the prefix is only set when output so that you can do long series of arithmetic with units without finding the best prefix each time.

I realize that having side effects in an object's toString method is probably the worst idea ever, but it achieves the desired results 99% of the time. But maybe it's possible we could refactor toString to work functionally, rather than changing the unit's state, perhaps even allowing us to remove the prefix state altogether. I'll think about this some more.

[josdejong]

I realize that having side effects in an object's toString method is probably the worst idea ever, but it achieves the desired results 99% of the time. But maybe it's possible we could refactor toString to work functionally, rather than changing the unit's state, perhaps even allowing us to remove the prefix state altogether. I'll think about this some more.

😄

It may indeed be better to have predictable output for toString. Maybe introduce an option similar to how we handle parentheses in toString outputs of expressions? (in that case you can pass options to toString: node.toString({parenthesis: 'auto' | 'keep' | 'all'})).

[josdejong]

Closing this issue in favor of #1483