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

Unable to compare with Infinity #61

Closed
lamberta opened this issue Nov 25, 2015 · 3 comments
Closed

Unable to compare with Infinity #61

lamberta opened this issue Nov 25, 2015 · 3 comments

Comments

@lamberta
Copy link

Philosophy of mathematics aside, I find comparisons against Infinity and -Infinity pretty useful, especially when initializing a cursor. Currently, this throws an error, but it'd be nice if it didn't:

123 < Infinity  //=> true

bigInt(123).compare(Infinity) //=>

Error: Invalid integer: Infinity
    at parseStringValue (./node_modules/big-integer/BigInteger.js:1116:33)
    at parseNumberValue (./node_modules/big-integer/BigInteger.js:1130:20)
    at parseValue (./node_modules/big-integer/BigInteger.js:1135:20)
    at SmallInteger.compare (./node_modules/big-integer/BigInteger.js:654:17)
    at repl:1:13
    at REPLServer.defaultEval (repl.js:248:27)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.<anonymous> (repl.js:412:12)
    at emitOne (events.js:82:20)
@peterolson
Copy link
Owner

Thanks for the suggestion. It's a tempting idea, but I need a little bit of time to consider whether it's worth including.

I'm guessing the typical use case for wanting to do this is something like finding a maximum or minimum value in an array, like this:

function findMinValue(arr) {
  var min = Infinity;
  for(var i = 0; i < arr.length; i++) {
    if(arr[i].lesser(min)) {
      min = arr[i];
    }
  }
  return min;
}

This does have the potentially unexpected behavior of returning Infinity when the array is empty. I feel that the following approach is more elegant

function findMinValue(arr) {
  var min = arr[0];
  for(var i = 1; i < arr.length; i++) {
    if(arr[i].lesser(min)) {
      min = arr[i];
    }
  }
  return min;
}

since it doesn't require an extra value not found in the array, and it returns undefined when the array is empty, which seems like a sensible output.

Basically, I'm saying that I'm having trouble coming up with a compelling use case where comparison with Infinity is the most elegant option. Do you have any examples?

I'm tempted to add it in anyways, since it's a simple change and I can't think of any downsides (other than a negligible increase in code size and calculation time), but first I'd like to see more examples of when it would be useful.

peterolson added a commit that referenced this issue Dec 1, 2015
peterolson added a commit that referenced this issue Dec 1, 2015
@peterolson
Copy link
Owner

I've decided to add this in, since it's a pretty simple change.

Note that I didn't add a mechanism for comparison with Infinity to the compareAbs method. This admittedly feels a little inconsistent, but I feel like such a feature would almost never be used. If anybody has a compelling argument for allowing comparison with Infinity using compareAbs, I can be persuaded.

@lamberta
Copy link
Author

lamberta commented Dec 1, 2015

Ah, thanks. Sorry, I forgot to get back here :|
Funny enough, my particular use case was working with the Twitter API. They store ids as int64, but also include an "id string" for JavaScript, and statuses are ordered so you page through them by comparing whichever is larger. I just needed to initialize my cursor to guarantee it was greater than anything they would give me, hence the initial comparison against Infinity. Maybe overkill, but was easy enough to implement.
So yeah, definitely not big deal to work around, just something that caught my eye and figured I post an issue.

For what it's worth, in vanilla JS Math.abs(Infinity) === Infinity so I would expect compareAbs to return the same as compare.

Anyway, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants