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

deepEquals behavior #78

Open
dpwrussell opened this issue Aug 21, 2016 · 1 comment
Open

deepEquals behavior #78

dpwrussell opened this issue Aug 21, 2016 · 1 comment

Comments

@dpwrussell
Copy link
Contributor

dpwrussell commented Aug 21, 2016

I'm a bit confused by the deepEquals behaviour here. As there is an option to enable deepObject equivalence I was not expecting objects to be equivalent (with deepObject=false) in this scenario.

const memoize = require('lru-memoize');

let multiply = (a, b, c) => {
  console.log('Multiplying', a, b, c);
  return a.val * b.val * c.val;
};

multiply = memoize(10)(multiply);

const a = {val:1};
const b = {val:2};
const c = {val:3};

const a2 = {val:1};

multiply(a,b,c)
multiply(a,b,c)
multiply(a2,b,c)

// Output
// Multiplying { val: 1 } { val: 2 } { val: 3 }

I was expecting that the multiple calculation would be run twice. Once for the first multiply(a,b,c) and again for the multiply(a2,b,c). The same is true for arrays.

I dug into the code a little and it looks like the intention is to penetrate 1 level deep by default?

As far as I know the arguments to a function will always be the array-like Arguments object, and I don't think that would ever be the same across function calls so it would make sense to me to start with something that handles the array specifically and then drops into the shallow or deepEquals possibilities.

Perhaps deepEquals and deepObject equals should be separate arguments, but I've reused deepObject in this example PR: #77

It should be easy to see the current behaviour for these unit tests by putting:

const cache = createCache(limit, deepEquals(equals, deepObjects));

back in.

@benjie
Copy link

benjie commented Mar 2, 2017

To add to this: nested arrays are evaluated deeply even if deepObjects is false (perhaps that's why it's called deepObjects?)

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

2 participants