Decimal instance chaining breaks when used on separate statements

[thanpolas]

  const decInst = new Decimal(0);
  decInst.plus('0.1');
  T.assertEqual('0.1', decInst.valueOf());

The above test fails, with a valueOf: '0', when expected should be '0.1'.

[josdejong]

Try decInst = decInst.plus('0.1') so you actually use the return value of .plus

[thanpolas]

I did

    let decimalRes = 0;
    decimalRes = Decimal.add(decimalRes, newVal);

but that's a workaround. This issue needs to be resolved, can't be the expected behaviour.

[josdejong]

I think you're misunderstanding the API. It's an immutable API.

Every method returns a new Decimal instance with the result, and the old instance is not changed: so in the next example, a and b will not change, and the result is returned as c.

const c = a.plus(b)

See docs for usage examples: https://github.com/MikeMcl/decimal.js#use (search for the word "immutable")

[thanpolas]

hmmm, that's kind of counter-intuitive:

add() is documented as a static method Decimal.add() and plus() is documented as an instance method new Decimal(val).plus(valb); --> https://mikemcl.github.io/decimal.js/#Dadd

Further examples make it blatantly obvious that the instance is mutable:

0.1 + 0.2                                // 0.30000000000000004
x = new Decimal(0.1)
y = x.plus(0.2)                          // '0.3'
new Decimal(0.7).plus(x).plus(y)         // '1.1'

The "instance" 0.7 gets mutated twice to become a value of 1.1. Whether you do that as part of chaining or on different statements, should not have any effect, if JS language norms were to be followed. To anyone that understands how instances work, they would expect my test case to work too.

But that is not the case.

So, I'd suggest that the authors either reconsider the instance methods and the instance's mutability or make sure it's blatantly obvious that they have hacked and altered the expected patterns and describe the new behaviours with fair notices on each instance.

[josdejong]

Thanos, the API actually is consistent and immutable. Reading your last comment I see you're still misunderstanding a thing or two. I'm not sure though if you're really interested in getting to understand it: the tone at the end of your comment comes across quite arrogant to me (I hope you don't mean it like that).

[thanpolas]

I am sorry if you understood my suggestion as arrogant.

I'd appreciate if you took the time, much like I did, to explain where I am misunderstanding a thing or two, much like I put the effort and time to do the same.

[MikeMcl]

@thanpolas

I don't know what you're smoking, but your example doesn't show any mutation.

All the methods of this library return a new Decimal instance (or a string, number etc.).

I don't know of any other big number libraries that don't use immutable instances. It's helpful to be able to use for example

zero = new Decimal(0)

and know that the value will not change.

Date objects and moment objects are two examples of mutable instances, but I can't think of many others, and note that:

Now, it is important to know that all of Moment’s maintainers agree that date and time types should be immutable. If we were sitting down and writing a new date and time library today, that is how we would make it.

[josdejong]

I am sorry if you understood my suggestion as arrogant.

👍 typically it's a good idea to first understand the library and next give advice instead of the other way around 😉

Yeah, momentjs is a good example: it's mutable API can result in nasty bugs, I've had troubles with that myself too. That's why immutable successors like luxon or date-fns are becoming more and more popular.

To go into the examples you give:

You can indeed do either Decimal.add(a, b) (static function), or a.plus(b) (method). The result is the same, and it is just a code style preference in the end. The documentation is just correct as far as I can see.

In the example new Decimal(0.7).plus(x).plus(y) the initial instance new Decimal(0.7) does not get mutated: plus returns a new Decimal instance without touching the original one. You can checkout whether a has a different value or not after calling a.plus(...):

const a = new Decimal(1)
const b = new Decimal(2)
const c = new Decimal(3)
const d = a.plus(b).plus(c)

a.plus(b) // this does not "do" anything effectively, since the newly created, returned Decimal is ignored

console.log(a.toString(), b.toString(), c.toString(), d.toString()) // 1, 2, 3, 6

// note that a is NOT changed itself by calling .plus(...) on it: it is immutable

Again, the docs gives a lot of examples to get to understand this. And just try out things in your developer console to verify how things work. I hope this helps.

[thanpolas]

const a = new Decimal(1)
const b = new Decimal(2)
const c = new Decimal(3)
const d = a.plus(b).plus(c)

Thank you @josdejong, this is the example that makes sense and should be in the docs.

@MikeMcl I am not smoking anything, you have a problem understanding feedback.

new Decimal(0.7).plus(x).plus(y)         // '1.1'

This can be read in two ways, either that the instance new Decimal(0.7) gets mutated and returns "1.1" or as per the example @josdejong laid out and I pasted above.

I happened to read it the first way, where the instance is mutated. That doesn't make me arrogant or smoking anything, it makes you condescending.

Enough with the adjectives though; the reality is that it is an honest mistake on my part that could have been easily avoided with better documentation.

Which has been one of my suggestions and still stands, amend your documentation so it's not ambiguous.

I will leave this issue open and as a request to give better examples on the documentation.

[MikeMcl]

Well, I agree with the bit about it being a mistake on your part.

Look again at the docs for the plus method

Returns a new Decimal whose value is the value of this Decimal plus x, rounded to precision significant digits using rounding mode rounding.

If you found the example ambiguous, then the above should have made things clear.

As, from the beginning of the Methods section, should:

A Decimal instance is immutable in the sense that it is not changed by its methods.

The code examples need to be read together with the text. I prefer to keep the code examples concise by not surrounding every expression with console.log, and I haven't had many complaints about their clarity.

If you want to have a go at making the documentation clearer then by all means do so, and I will certainly review any pull request you submit.

The reference to smoking was intended to be a light-hearted reference to your profile picture.