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

Non numbers shouldn't be used for the almost-equal check #18

Open
TN1ck opened this issue Apr 5, 2016 · 4 comments
Open

Non numbers shouldn't be used for the almost-equal check #18

TN1ck opened this issue Apr 5, 2016 · 4 comments

Comments

@TN1ck
Copy link

TN1ck commented Apr 5, 2016

Writing a assertion like the following, will fail, because the library expects only numbers. I expected it only looks at the numbers, not at strings or other objects.

expect({key: 0.8, bar: 'foo'}).to.almost.eql({key: 0.8, bar: 'foo'});
@keithamus
Copy link
Member

Hey @TN1ck thanks for the issue.

I'm not sure we should change this behaviour. I think it'd be better for you to change your assertion to only assert on the numeric keys. Thoughts?

@TN1ck
Copy link
Author

TN1ck commented Apr 5, 2016

Yes, it's not clear what the correct way should be. But it feels strange when equal values aren't considered as equal, a error-message with value {x} is not a number and is always considered unequal, only use this library to compare collections of numbers would help a lot, I think it's a common pattern to have some strings inside a numeric data-collection.
For my usage it was really simple to provide a collection with the non-numbers omitted, so it was a non-issue for me after i knew what the problem was.
And thanks for the lib, it's really useful!

@keithamus
Copy link
Member

That error message sounds like it could indeed be useful. Do you fancy making a pull request to this effect?

@glen-nu
Copy link

glen-nu commented Feb 13, 2017

I had this issue but I wasn't able to find a good way to pass in only numbers/objects. I ended up patching the deepAlmostEqual function to cover the non-number and non-object cases because I really needed it to work with strings. I've created a branch called all-type-comparison with some tests for this.

exports.deepAlmostEqual = function(a,b,precision){
  var tol = tolerance(precision);
  function deepEql (act, exp) {
    if (Object(act) === act){
      for (var k in act) {
        if (typeof act[k] === typeof exp[k] && (typeof act[k] === 'number'
                                                || typeof act[k] === 'object')) {
          if (!(deepEql(act[k], exp[k]))) {
            return false;
          }
        } else {
          if (act[k] !== exp[k]) {
            return false;
          }
        }
      }
      return true;
    } else {
      return Math.abs(act - exp) < tol;
    }
  }

  return deepEql(a,b);
};

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

No branches or pull requests

3 participants