Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extends API to support compareTo aliases (===, >, >=, etc...) #73

Closed
d-mon- opened this issue May 12, 2022 · 5 comments · Fixed by #99
Closed

Extends API to support compareTo aliases (===, >, >=, etc...) #73

d-mon- opened this issue May 12, 2022 · 5 comments · Fixed by #99
Labels
enhancement New feature or request

Comments

@d-mon-
Copy link
Contributor

d-mon- commented May 12, 2022

Is your feature request related to a problem? Please describe.
Relying on compareTo is quite verbose and not very comfortable to read or use in my humble opinion.
I think this library will greatly benefit by adding aliases for each comparaison operators.

Describe the solution you'd like
Ideally, I would like to extends the API to support gt, gte, lt, lte as aliases of compareTo (for reference: bignumber, big)
I would also like to make equals less strict by making it an alias for === 0 and rename the current method to strictEquals when scale matter

   if (x.equals(y)) // ===
   if (!x.equals(y)) // !==
   if (x.strictEquals(y)) // equals in value & scale
   if(x.gt(y))  // >
   if(x.gte(y)) // >=
   if(x.lt(y))   // <
   if(x.lte(y))  // <=

If the intention is to keep the API closely related to the JAVA implementation, it is also perfectly fine to find a different alternative for the === alias. (ex: sameAs, similar, equivalent, eq, etc...)

I'm also considering adding notEquals to the list

Describe alternatives you've considered
Create a new Compare utils

class Comparator {
  constructor(private value: BigDecimal) {}

  equals(value: BigDecimal) {
    return this.value.compareTo(value) === 0;
  }

  notEquals(value: BigDecimal) {
    return this.value.compareTo(value) !== 0;
  }

  gt(value: BigDecimal) {
    return this.value.compareTo(value) > 0;
  }

  gte(value: BigDecimal) {
    return this.value.compareTo(value) >= 0;
  }

  lt(value: BigDecimal) {
    return this.value.compareTo(value) < 0;
  }

  lte(value: BigDecimal) {
    return this.value.compareTo(value) <= 0;
  }
}

function Compare(value: BigDecimal): Comparator {
  return new Comparator(value);
}

// if(Compare(x).gte(y))

I'm currently using this util in my own projects to avoid extending the bigdecimal.js class

@d-mon- d-mon- added the enhancement New feature or request label May 12, 2022
@srknzl
Copy link
Owner

srknzl commented May 12, 2022

I liked the idea of extending the API. We can certainly add them.

If the intention is to keep the API closely related to the JAVA implementation, it is also perfectly fine to find a different alternative for the === alias. (ex: sameAs, eq, etc...)

Since this library is ported from Java, diverging from it too much will cause confusion. Let me think what we can do about equals.

@d-mon-
Copy link
Contributor Author

d-mon- commented Jun 13, 2022

Hi!
I gave some more thoughts on the equals issue.
Concerning my current stance, I think having equals and strictEquals is still a better approach for this library even if it means to draw apart from Java. It would require adding an explanation (and probably a link) to map the JS strictEquals to the Java equals.

I also think the JS community is already very familiar with the difference between == vs === , (assert equals, etc...) which is a recurring problem in weakly typed language like JS.
And so mapping == to equals and === to strictEquals seems more fitting.

Another point I would like to add: If operator overloading is added to Javascript one day, this will allow the following notations:

Big("2.0") == 2 // true 
Big("2.0") == Big("2.00") // true 
Big("2.0") >= 2 // true

Big("2.0") === 2 // false
Big("2.0") === Big("2.00") // false
Big("2.00") === Big("2.00") // true

It won't happen soon though as the proposal will take some time before even being considered for stage 2 (the maintainer seems to be taking a break from TC39). -- and Decimal will be released by then.


I also added a new suggestion to the list of alternatives names: equivalent (equiv) which I think represent the == idea above pretty well (better than sameAs in my opinion)

$$ \dfrac{123}{100} \equiv \dfrac{1230}{1000} \equiv \dfrac{12300}{10000}\ ... $$

@srknzl
Copy link
Owner

srknzl commented Jun 19, 2022

I think we can add eq as an alias to equals and make equals method less strict, i.e only compare the value, not the scale. This is a breaking change though.

I am not sure with the name though. I think equalsStrict is a better name than strictEquals, because with autocompletion, both will be seen by the users.equalsIncludingScale is definitely feels a bit long. equalsWithScale is another alternative. However, equalsStrict may be a better name. The only issue with it is that the intention about being strict may not be obvious by reading its name. I like method names with clear intentions like equalsSameScale.

@d-mon-
Copy link
Contributor Author

d-mon- commented Jul 5, 2022

First of all, I would like to come back on my previous comment.
After reading this issue, I think my example deosn't work well with the current usage of JS, it should probably become:

Big("2.0") === 2 // false 
Big("2.0") === Big("2.00") // true 
Big("2.00") === Big("2.00") // true
Big("2.0") ====  Big("2.00") // false 
Big("2.00") ====  Big("2.00") // true 

As most linter recommend === over == by default. And I think developers would also prefer 2.0 to be equal to 2.00 in most cases. It seems logical to avoid checking the scale when using ===.
The new operator ==== in the list above would then do the same thing as === + compare the scale just like the current equals.


Anyway, first of all, I really like your point on autocompletion and definitely share your point of view on having a method names with clear intentions.

Of all the propositions listed so far, I think equalsStrict might be the best as it infers a stronger check on BigDecimal's inner values compared to equals.

equalsWithScale is also nice. But I'm a bit concerned as we can also interpret the new equals being the same as equalsWithoutScale (I might overthink this one). I was also thinking of simply replacing Scale by Precision which should make it easier to understand ?

Concerning equalsSameScale, in addition of having two words sharing the same meaning, At first glance it can be mistaken to a function used only to compare scale values.

I have a few other names to suggest very similar to equalsStrict:

  • equalsPrecise(ly)
  • equalsExact(ly)

@d-mon-
Copy link
Contributor Author

d-mon- commented Aug 16, 2022

I just received another proposal for the naming to mitigate the breaking change on equals.
The new name suggested was looselyEquals or sameValue in opposition to strict. (alternative names: equalsLoosely / equalsLoose / looseEquals).
References:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness
https://tc39.es/ecma262/#sec-islooselyequal
https://tc39.es/ecma262/#sec-samevalue

@srknzl srknzl linked a pull request Sep 30, 2022 that will close this issue
@srknzl srknzl closed this as completed in #99 Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants