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.

Sign up for GitHub

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

Jump to bottom

Memoize not working properly with NaN #101

Closed
ohoho7 opened this issue Nov 27, 2020 · 4 comments
Closed

Memoize not working properly with NaN #101

ohoho7 opened this issue Nov 27, 2020 · 4 comments

Comments

@ohoho7
Copy link

ohoho7 commented Nov 27, 2020 •
edited
Loading

Doc has statement:
Tested with all built in JavaScript types.
But it is not working properly with NaN value, example:

let i = 0;
function getIncrementedNumber(parameter) {
    return i  ;
}
const memoizedIncrementedNumber = memoize(getIncrementedNumber);
const value = NaN;
console.log(memoizedIncrementedNumber(value)); // log 0
console.log(memoizedIncrementedNumber(value)); // log 1
@alexreardon
Copy link
Owner

We will probably need to add an NaN check. NaN !== NaN

@alexreardon
Copy link
Owner

this check is not correct for NaN:

for (let i = 0; i < newInputs.length; i  ) {
    // using shallow equality check
    if (newInputs[i] !== lastInputs[i]) {
      return false;
    }
  }

@alexreardon alexreardon changed the title Memoize not working properly with number type Memoize not working properly with NaN Dec 2, 2020
@Ayub-Begimkulov Ayub-Begimkulov mentioned this issue Feb 7, 2021
Closed
@alexreardon
Copy link
Owner

Sorry for my delay in this. I have been overwhelmed with other things. I want to get this in, but I want to do some benchmarking of different approaches first

@alexreardon alexreardon mentioned this issue Apr 12, 2021
Merged
@alexreardon
Copy link
Owner

Closed by #112

@snyk-bot snyk-bot mentioned this issue Mar 3, 2023
Open
@dmitrymarokhonov dmitrymarokhonov mentioned this issue May 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@alexreardon @ohoho7